Skip to content

Commit

Permalink
Better defer and subpath checks for unpack
Browse files Browse the repository at this point in the history
Added a check if the tar header is a subpath of the
context. Improved the defer to handle errors on Close.
  • Loading branch information
gorkem committed Nov 27, 2024
1 parent 95d44a2 commit ec4fb37
Showing 1 changed file with 15 additions and 3 deletions.
18 changes: 15 additions & 3 deletions pkg/cmd/unpack/unpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func unpackLayer(ctx context.Context, store content.Storage, desc ocispec.Descri
return nil
}

func extractTar(tr *tar.Reader, dir string, overwrite bool, logger *output.ProgressLogger) error {
func extractTar(tr *tar.Reader, dir string, overwrite bool, logger *output.ProgressLogger) (err error) {
for {
header, err := tr.Next()
if err == io.EOF {
Expand All @@ -247,6 +247,11 @@ func extractTar(tr *tar.Reader, dir string, overwrite bool, logger *output.Progr
return err
}
outPath := filepath.Join(dir, header.Name)
// Check if the outPath is within the target directory
_, _, err = filesystem.VerifySubpath(dir, outPath)
if err != nil {
return fmt.Errorf("illegal file path: %s: %w", outPath, err)
}

switch header.Typeflag {
case tar.TypeDir:
Expand Down Expand Up @@ -276,8 +281,15 @@ func extractTar(tr *tar.Reader, dir string, overwrite bool, logger *output.Progr
if err != nil {
return fmt.Errorf("failed to create file %s: %w", outPath, err)
}
defer file.Close()

defer func() {
if errClose := file.Close(); errClose != nil {
if err == nil {
err = fmt.Errorf("failed to close log file: %w", errClose)
} else {
err = fmt.Errorf("%v; failed to close log file: %w", err, errClose)
}
}
}()
written, err := io.Copy(file, tr)
if err != nil {
return fmt.Errorf("failed to write file %s: %w", outPath, err)
Expand Down

0 comments on commit ec4fb37

Please sign in to comment.