fix(summary): arbitrary file access during archive extraction zipslip #39896

opsysdebug · 2025-06-04T14:06:49Z

Lines 227 to 253 in 94ac65e

    
           for _, file := range r.File { 
        
           	destPath := filepath.Join(destDir, file.Name) 
        
           	if file.FileInfo().IsDir() { 
        
           		// Create directories 
        
           		if err := os.MkdirAll(destPath, os.ModePerm); err != nil { 
        
           			return fmt.Errorf("failed to create directory %s: %w", destPath, err) 
        
           		} 
        
           		continue 
        
           	} 
        
           	// Extract files 
        
           	destFile, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, file.Mode()) 
        
           	if err != nil { 
        
           		return fmt.Errorf("failed to create file %s: %w", destPath, err) 
        
           	} 
        
           	srcFile, err := file.Open() 
        
           	if err != nil { 
        
           		destFile.Close() 
        
           		return fmt.Errorf("failed to open file in ZIP archive %s: %w", file.Name, err) 
        
           	} 
        
           	_, err = io.Copy(destFile, srcFile) 
        
           	destFile.Close() 
        
           	srcFile.Close() 
        
           	if err != nil { 
        
           		return fmt.Errorf("failed to extract file %s: %w", file.Name, err) 
        
           	} 
        
           }

Fix the issue need to validate the file.Name field from the ZIP archive before using it to construct file paths. Specifically:

Ensure that file.Name does not contain directory traversal elements (..) or absolute paths.
Use filepath.Clean to normalize the path and check that the resulting path is within the intended destDir.

The fix involves:

Adding a check to ensure file.Name does not contain .. or result in paths outside destDir.
Using filepath.Clean to sanitize the path.
Rejecting or skipping files with invalid paths.

Zip Slip Vulnerability

For first time contributors, read Submitting a pull request
All code is covered by unit and/or runtime tests where feasible.
All commits contain a well written commit description including a title,
description and a Fixes: #XXX line if the commit addresses a particular
GitHub issue.
If your commit description contains a Fixes: <commit-id> tag, then
please add the commit author[s] as reviewer[s] to this issue.
All commits are signed off. See the section Developer’s Certificate of Origin
Provide a title or release-note blurb suitable for the release notes.
Are you a user of Cilium? Please add yourself to the Users doc
Thanks for contributing!

Fixes: #issue-number

<!-- Enter the release note text here if needed or remove this section! -->

asauber

Thank you for the contribution. I don't see any downside to these additional checks. Thankfully, this unzip codepath is used only within the context of temporary workflow runners.

Will run tests and let's look for everything to pass.

asauber · 2025-06-05T02:11:56Z

/test

joestringer · 2025-06-25T15:45:16Z

We reviewed the list of stuck PRs during the community meeting and this PR came up. It looks like the PR stalled due to test failures; I've retriggered them to hopefully unblock that. Additionally, due to team changes the cilium-cli approval is no longer valid. I've re-requested review from @cilium/cli to help shepherd this PR into the tree. Thanks for your contribution.

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>

tklauser · 2025-06-26T14:49:51Z

/test

opsysdebug requested a review from a team as a code owner June 4, 2025 14:06

opsysdebug requested a review from asauber June 4, 2025 14:06

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 4, 2025

github-actions bot added cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary kind/community-contribution This was a contribution made by a community member. labels Jun 4, 2025

asauber reviewed Jun 5, 2025

View reviewed changes

asauber added the release-note/ci This PR makes changes to the CI. label Jun 5, 2025

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 5, 2025

joestringer enabled auto-merge June 25, 2025 15:42

joestringer requested review from a team and tklauser and removed request for a team June 25, 2025 15:42

tklauser approved these changes Jun 26, 2025

View reviewed changes

Update summary.go

550878c

Signed-off-by: Zeroday BYTE <github@zerodaysec.org>

tklauser force-pushed the patch-1 branch from 7c7a451 to 550878c Compare June 26, 2025 14:43

joestringer added this pull request to the merge queue Jun 26, 2025

Merged via the queue into cilium:main with commit 240441f Jun 26, 2025
68 checks passed

maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Jun 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(summary): arbitrary file access during archive extraction zipslip #39896

fix(summary): arbitrary file access during archive extraction zipslip #39896

Uh oh!

opsysdebug commented Jun 4, 2025 •

edited

Loading

Uh oh!

asauber left a comment

Uh oh!

asauber commented Jun 5, 2025

Uh oh!

joestringer commented Jun 25, 2025

Uh oh!

tklauser commented Jun 26, 2025

Uh oh!

Uh oh!

Uh oh!

	for _, file := range r.File {
	destPath := filepath.Join(destDir, file.Name)
	if file.FileInfo().IsDir() {
	// Create directories
	if err := os.MkdirAll(destPath, os.ModePerm); err != nil {
	return fmt.Errorf("failed to create directory %s: %w", destPath, err)
	}
	continue
	}

	// Extract files
	destFile, err := os.OpenFile(destPath, os.O_WRONLY\|os.O_CREATE\|os.O_TRUNC, file.Mode())
	if err != nil {
	return fmt.Errorf("failed to create file %s: %w", destPath, err)
	}
	srcFile, err := file.Open()
	if err != nil {
	destFile.Close()
	return fmt.Errorf("failed to open file in ZIP archive %s: %w", file.Name, err)
	}
	_, err = io.Copy(destFile, srcFile)
	destFile.Close()
	srcFile.Close()
	if err != nil {
	return fmt.Errorf("failed to extract file %s: %w", file.Name, err)
	}
	}

fix(summary): arbitrary file access during archive extraction zipslip #39896

fix(summary): arbitrary file access during archive extraction zipslip #39896

Uh oh!

Conversation

opsysdebug commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asauber left a comment

Choose a reason for hiding this comment

Uh oh!

asauber commented Jun 5, 2025

Uh oh!

joestringer commented Jun 25, 2025

Uh oh!

tklauser commented Jun 26, 2025

Uh oh!

Uh oh!

Uh oh!

opsysdebug commented Jun 4, 2025 •

edited

Loading