From 4c7d9ccef76297f2794e67ce54f0d2fe0fcf56a5 Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Thu, 9 Jul 2020 11:53:52 -0400 Subject: [PATCH] simplify unzip; update java error statements --- imgbom/cataloger/java/archive_filename.go | 43 ++++++------- imgbom/cataloger/java/java_manifest.go | 4 +- internal/file/ziputil.go | 77 ++++++++++++----------- 3 files changed, 63 insertions(+), 61 deletions(-) diff --git a/imgbom/cataloger/java/archive_filename.go b/imgbom/cataloger/java/archive_filename.go index 529508655..8d1c1a465 100644 --- a/imgbom/cataloger/java/archive_filename.go +++ b/imgbom/cataloger/java/archive_filename.go @@ -5,6 +5,8 @@ import ( "regexp" "strings" + "github.com/anchore/imgbom/internal/log" + "github.com/anchore/imgbom/imgbom/pkg" ) @@ -15,25 +17,16 @@ import ( var versionPattern = regexp.MustCompile(`(?P.+)-(?P(\d+\.)?(\d+\.)?(\*|\d+)(-[a-zA-Z0-9\-\.]+)*)`) type archiveFilename struct { - raw string + raw string + fields []map[string]string } func newJavaArchiveFilename(raw string) archiveFilename { - return archiveFilename{ - raw: raw, - } -} - -func (a archiveFilename) normalize() string { // trim the file extension and remove any path prefixes - return strings.TrimSuffix(filepath.Base(a.raw), "."+a.extension()) -} - -func (a archiveFilename) fields() []map[string]string { - name := a.normalize() + name := strings.TrimSuffix(filepath.Base(raw), filepath.Ext(raw)) matches := versionPattern.FindAllStringSubmatch(name, -1) - items := make([]map[string]string, 0) + fields := make([]map[string]string, 0) for _, match := range matches { item := make(map[string]string) for i, name := range versionPattern.SubexpNames() { @@ -41,9 +34,13 @@ func (a archiveFilename) fields() []map[string]string { item[name] = match[i] } } - items = append(items, item) + fields = append(fields, item) + } + + return archiveFilename{ + raw: raw, + fields: fields, } - return items } func (a archiveFilename) extension() string { @@ -62,23 +59,21 @@ func (a archiveFilename) pkgType() pkg.Type { } func (a archiveFilename) version() string { - fields := a.fields() - - // there should be only one version, if there is more or less then something is wrong - if len(fields) != 1 { + if len(a.fields) > 1 { + log.Errorf("discovered multiple name-version pairings from %q: %+v", a.raw, a.fields) + return "" + } else if len(a.fields) < 1 { return "" } - return fields[0]["version"] + return a.fields[0]["version"] } func (a archiveFilename) name() string { - fields := a.fields() - // there should be only one name, if there is more or less then something is wrong - if len(fields) != 1 { + if len(a.fields) != 1 { return "" } - return fields[0]["name"] + return a.fields[0]["name"] } diff --git a/imgbom/cataloger/java/java_manifest.go b/imgbom/cataloger/java/java_manifest.go index a953eebde..7dea2bdbf 100644 --- a/imgbom/cataloger/java/java_manifest.go +++ b/imgbom/cataloger/java/java_manifest.go @@ -54,7 +54,7 @@ func parseJavaManifest(reader io.Reader) (*pkg.JavaManifest, error) { } if err := mapstructure.Decode(manifestMap, &manifest); err != nil { - return nil, fmt.Errorf("unable parse java manifest: %w", err) + return nil, fmt.Errorf("unable to parse java manifest: %w", err) } return &manifest, nil @@ -73,7 +73,7 @@ func newPackageFromJavaManifest(virtualPath, archivePath string, fileManifest fi // fetch the manifest file contents, err := file.ExtractFilesFromZip(archivePath, manifestMatches...) if err != nil { - return nil, fmt.Errorf("unable to extract java manifests: %w", err) + return nil, fmt.Errorf("unable to extract java manifests (%s): %w", virtualPath, err) } // parse the manifest file into a rich object diff --git a/internal/file/ziputil.go b/internal/file/ziputil.go index c07727ea4..249c25a30 100644 --- a/internal/file/ziputil.go +++ b/internal/file/ziputil.go @@ -83,7 +83,6 @@ func ExtractFilesFromZip(archivePath string, paths ...string) (map[string]string return results, nil } -// nolint:funlen func UnzipToDir(archivePath, targetDir string) error { zipReader, err := zip.OpenReader(archivePath) if err != nil { @@ -106,46 +105,54 @@ func UnzipToDir(archivePath, targetDir string) error { return fmt.Errorf("potential zip slip attack: %q", expandedFilePath) } - zippedFile, err := file.Open() + err = extractSingleFile(file, expandedFilePath, archivePath) if err != nil { - return fmt.Errorf("unable to read file=%q from zip=%q: %w", file.Name, archivePath, err) + return err } + } + return nil +} - if file.FileInfo().IsDir() { - err = os.MkdirAll(expandedFilePath, file.Mode()) - if err != nil { - return fmt.Errorf("unable to create dir=%q from zip=%q: %w", expandedFilePath, archivePath, err) - } - } else { - // Open an output file for writing - outputFile, err := os.OpenFile( - expandedFilePath, - os.O_WRONLY|os.O_CREATE|os.O_TRUNC, - file.Mode(), - ) - if err != nil { - return fmt.Errorf("unable to create dest file=%q from zip=%q: %w", expandedFilePath, archivePath, err) - } +func extractSingleFile(file *zip.File, expandedFilePath, archivePath string) error { + zippedFile, err := file.Open() + if err != nil { + return fmt.Errorf("unable to read file=%q from zip=%q: %w", file.Name, archivePath, err) + } - // limit the zip reader on each file read to prevent decompression bomb attacks - numBytes, err := io.Copy(outputFile, io.LimitReader(zippedFile, readLimit)) - if numBytes >= readLimit || errors.Is(err, io.EOF) { - return fmt.Errorf("zip read limit hit (potential decompression bomb attack)") - } - if err != nil { - return fmt.Errorf("unable to copy source=%q to dest=%q for zip=%q: %w", file.Name, outputFile.Name(), archivePath, err) - } - - err = outputFile.Close() - if err != nil { - return fmt.Errorf("unable to close dest file=%q from zip=%q: %w", outputFile.Name(), archivePath, err) - } - } - - err = zippedFile.Close() + if file.FileInfo().IsDir() { + err = os.MkdirAll(expandedFilePath, file.Mode()) if err != nil { - return fmt.Errorf("unable to close source file=%q from zip=%q: %w", file.Name, archivePath, err) + return fmt.Errorf("unable to create dir=%q from zip=%q: %w", expandedFilePath, archivePath, err) } + } else { + // Open an output file for writing + outputFile, err := os.OpenFile( + expandedFilePath, + os.O_WRONLY|os.O_CREATE|os.O_TRUNC, + file.Mode(), + ) + if err != nil { + return fmt.Errorf("unable to create dest file=%q from zip=%q: %w", expandedFilePath, archivePath, err) + } + + // limit the zip reader on each file read to prevent decompression bomb attacks + numBytes, err := io.Copy(outputFile, io.LimitReader(zippedFile, readLimit)) + if numBytes >= readLimit || errors.Is(err, io.EOF) { + return fmt.Errorf("zip read limit hit (potential decompression bomb attack)") + } + if err != nil { + return fmt.Errorf("unable to copy source=%q to dest=%q for zip=%q: %w", file.Name, outputFile.Name(), archivePath, err) + } + + err = outputFile.Close() + if err != nil { + return fmt.Errorf("unable to close dest file=%q from zip=%q: %w", outputFile.Name(), archivePath, err) + } + } + + err = zippedFile.Close() + if err != nil { + return fmt.Errorf("unable to close source file=%q from zip=%q: %w", file.Name, archivePath, err) } return nil }