MIC: Add support for self-contained initrd for LiveOS ISO images #233
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gmileka
changed the title
gmileka
force-pushed
the
user/gmileka/only-initrd
branch
from
3232c46 to
19b21eb
Compare
gmileka
force-pushed
the
user/gmileka/only-initrd
branch
from
19b21eb to
4b3f6ed
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return fmt.Errorf("error reading tar: %w", err) | ||
| } | ||
|
|
||
| target := filepath.Join(outputDir, header.Name) |
Check failure
Code scanning / CodeQL
Arbitrary file write extracting an archive containing symbolic links High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
To fix the issue, we need to ensure that paths extracted from the archive are validated and resolved to prevent directory traversal or symbolic link exploitation. Specifically:
- Use
filepath.EvalSymlinksto resolve any symbolic links in the target directory before validating paths. - Ensure that both
header.Nameandheader.Linknameare checked to confirm they do not escape the intended extraction directory. - Reject any paths that resolve outside the
outputDir.
Changes will be made to the expandTarGzArchive function:
- Add validation logic for
header.Nameandheader.Linknameusingfilepath.EvalSymlinks. - Modify the symbolic link creation logic to ensure the resolved paths are safe.
Suggested changeset
1
toolkit/tools/pkg/imagecustomizerlib/liveosisoimages.go
| @@ -620,3 +620,11 @@ | ||
|
|
||
| target := filepath.Join(outputDir, header.Name) | ||
| // Resolve and validate the target path | ||
| resolvedTarget, err := filepath.EvalSymlinks(filepath.Join(outputDir, header.Name)) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to resolve path (%s):\n%w", header.Name, err) | ||
| } | ||
| relPath, err := filepath.Rel(outputDir, resolvedTarget) | ||
| if err != nil || strings.HasPrefix(filepath.Clean(relPath), "..") { | ||
| return fmt.Errorf("invalid path (%s) escaping output directory", resolvedTarget) | ||
| } | ||
|
|
||
| @@ -624,12 +632,12 @@ | ||
| case tar.TypeDir: | ||
| if err := os.MkdirAll(target, os.FileMode(header.Mode)); err != nil { | ||
| return fmt.Errorf("failed to create folder (%s)\n%w", target, err) | ||
| if err := os.MkdirAll(resolvedTarget, os.FileMode(header.Mode)); err != nil { | ||
| return fmt.Errorf("failed to create folder (%s)\n%w", resolvedTarget, err) | ||
| } | ||
| case tar.TypeReg: | ||
| if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil { | ||
| return fmt.Errorf("failed to create parent folder for (%s)\n%w", target, err) | ||
| if err := os.MkdirAll(filepath.Dir(resolvedTarget), 0755); err != nil { | ||
| return fmt.Errorf("failed to create parent folder for (%s)\n%w", resolvedTarget, err) | ||
| } | ||
| outFile, err := os.Create(target) | ||
| outFile, err := os.Create(resolvedTarget) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create (%s):\n%w", target, err) | ||
| return fmt.Errorf("failed to create (%s):\n%w", resolvedTarget, err) | ||
| } | ||
| @@ -638,3 +646,3 @@ | ||
| if err != nil { | ||
| return fmt.Errorf("failed to copy (%s) from archive:\n%w", target, err) | ||
| return fmt.Errorf("failed to copy (%s) from archive:\n%w", resolvedTarget, err) | ||
| } | ||
| @@ -642,11 +650,20 @@ | ||
|
|
||
| if err := os.Chmod(target, os.FileMode(header.Mode)); err != nil { | ||
| return fmt.Errorf("failed to set permissions (%d) on (%s):\n%w", os.FileMode(header.Mode), target, err) | ||
| if err := os.Chmod(resolvedTarget, os.FileMode(header.Mode)); err != nil { | ||
| return fmt.Errorf("failed to set permissions (%d) on (%s):\n%w", os.FileMode(header.Mode), resolvedTarget, err) | ||
| } | ||
| case tar.TypeSymlink: | ||
| if err := os.Symlink(header.Linkname, target); err != nil { | ||
| return fmt.Errorf("failed to create symbolic link (%s):\n%w", target, err) | ||
| // Resolve and validate the link target | ||
| resolvedLinkTarget, err := filepath.EvalSymlinks(filepath.Join(outputDir, header.Linkname)) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to resolve link target (%s):\n%w", header.Linkname, err) | ||
| } | ||
| relLinkPath, err := filepath.Rel(outputDir, resolvedLinkTarget) | ||
| if err != nil || strings.HasPrefix(filepath.Clean(relLinkPath), "..") { | ||
| return fmt.Errorf("invalid link target (%s) escaping output directory", resolvedLinkTarget) | ||
| } | ||
| if err := os.Symlink(resolvedLinkTarget, resolvedTarget); err != nil { | ||
| return fmt.Errorf("failed to create symbolic link (%s):\n%w", resolvedTarget, err) | ||
| } | ||
| default: | ||
| return fmt.Errorf("failed to process unsupported file type in archive (%s): (%v)", target, header.Typeflag) | ||
| return fmt.Errorf("failed to process unsupported file type in archive (%s): (%v)", resolvedTarget, header.Typeflag) | ||
| } |
Copilot is powered by AI and may make mistakes. Always verify output.
|
|
||
| tr := tar.NewReader(gzr) | ||
| for { | ||
| header, err := tr.Next() |
Check failure
Code scanning / CodeQL
Arbitrary file access during archive extraction ("Zip Slip") High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
To fix the issue, we need to validate the header.Name field before using it to construct file paths. Specifically, we should ensure that the path does not contain directory traversal elements (..) or absolute paths. This can be achieved by checking the sanitized path against the intended outputDir to ensure it remains within the expected directory.
Steps to fix:
- Use
filepath.Cleanto sanitize theheader.Namefield. - Ensure the resulting path is within the
outputDirby comparing the base directory of the constructed path withoutputDir. - Reject or skip entries that fail these checks.
Suggested changeset
1
toolkit/tools/pkg/imagecustomizerlib/liveosisoimages.go
| @@ -620,3 +620,13 @@ | ||
|
|
||
| target := filepath.Join(outputDir, header.Name) | ||
| // Sanitize and validate the path | ||
| cleanedName := filepath.Clean(header.Name) | ||
| if strings.Contains(cleanedName, "..") || filepath.IsAbs(cleanedName) { | ||
| logger.Log.Warnf("Skipping potentially unsafe archive entry: %s", header.Name) | ||
| continue | ||
| } | ||
| target := filepath.Join(outputDir, cleanedName) | ||
| if !strings.HasPrefix(target, filepath.Clean(outputDir)+string(os.PathSeparator)) { | ||
| logger.Log.Warnf("Skipping entry outside output directory: %s", header.Name) | ||
| continue | ||
| } | ||
|
|
Copilot is powered by AI and may make mistakes. Always verify output.
| return fmt.Errorf("failed to set permissions (%d) on (%s):\n%w", os.FileMode(header.Mode), target, err) | ||
| } | ||
| case tar.TypeSymlink: | ||
| if err := os.Symlink(header.Linkname, target); err != nil { |
Check failure
Code scanning / CodeQL
Arbitrary file write extracting an archive containing symbolic links High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix the issue, the code must validate and resolve symbolic links before creating them. Specifically:
- Use
filepath.EvalSymlinksto resolve theheader.Linknameand ensure it points to a location within theoutputDir. - Check the resolved path against the
outputDirusingfilepath.Relto confirm it does not escape the intended directory. - Reject symbolic links that fail these checks to prevent arbitrary file writes outside the target directory.
The changes will be applied to the expandTarGzArchive function, specifically within the case tar.TypeSymlink block. Necessary imports and validations will be added.
Suggested changeset
1
toolkit/tools/pkg/imagecustomizerlib/liveosisoimages.go
| @@ -646,3 +646,13 @@ | ||
| case tar.TypeSymlink: | ||
| if err := os.Symlink(header.Linkname, target); err != nil { | ||
| // Resolve the symbolic link target and validate it | ||
| resolvedLinkname, err := filepath.EvalSymlinks(filepath.Join(outputDir, header.Linkname)) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to resolve symbolic link target (%s):\n%w", header.Linkname, err) | ||
| } | ||
| relPath, err := filepath.Rel(outputDir, resolvedLinkname) | ||
| if err != nil || strings.HasPrefix(filepath.Clean(relPath), "..") { | ||
| return fmt.Errorf("symbolic link target (%s) escapes the output directory (%s)", resolvedLinkname, outputDir) | ||
| } | ||
| // Create the symbolic link | ||
| if err := os.Symlink(resolvedLinkname, target); err != nil { | ||
| return fmt.Errorf("failed to create symbolic link (%s):\n%w", target, err) |
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist