fix: harden UPX exec calls and limit file name length (#1342)

Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>
Co-authored-by: Steve Beattie <steve+github@nxnw.org>

Author: egibs

pkg/archive/upx.go

@@ -31,17 +31,24 @@ func ExtractUPX(ctx context.Context, d, f string) error {
 		return fmt.Errorf("failed to stat file: %w", err)
 	}
 
-	gf, err := os.Open(f)
-	if err != nil {
-		return fmt.Errorf("failed to open file: %w", err)
+	base := filepath.Base(f)
+	if strings.HasPrefix(base, "-") {
+		return fmt.Errorf("file name begins with '-': %q", base)
+	}
+	if len(base) > 255 {
+		return fmt.Errorf("file name exceeds 255 characters")
 	}
-	defer gf.Close()
 
 	target := filepath.Join(d, filepath.Base(f))
 	if !IsValidPath(target, d) {
 		return fmt.Errorf("invalid file path: %s", target)
 	}
 
+	absTarget, err := filepath.Abs(target)
+	if err != nil {
+		return fmt.Errorf("failed to get absolute path: %w", err)
+	}
+
 	tf, err := os.ReadFile(f)
 	if err != nil {
 		return fmt.Errorf("failed to read file: %w", err)
@@ -52,18 +59,18 @@ func ExtractUPX(ctx context.Context, d, f string) error {
 		return fmt.Errorf("failed to write file: %w", err)
 	}
 
-	cmd := exec.CommandContext(ctx, "upx", "-d", "-k", target)
+	cmd := exec.CommandContext(ctx, "upx", "-d", "-k", "--", absTarget)
 	output, err := cmd.CombinedOutput()
 	if err != nil {
-		os.Remove(target)
+		os.Remove(absTarget)
 		return fmt.Errorf("failed to decompress upx file: %w, output: %s", err, output)
 	}
 
 	if !strings.Contains(string(output), "Decompressed") && !strings.Contains(string(output), "Unpacked") {
-		os.Remove(target)
+		os.Remove(absTarget)
 		return fmt.Errorf("upx decompression might have failed: %s", output)
 	}
 
-	logger.Debug("successfully decompressed upx file", "output", string(output), "target", target)
+	logger.Debug("successfully decompressed upx file", "output", string(output), "target", absTarget)
 	return nil
 }

pkg/programkind/programkind.go

@@ -249,7 +249,20 @@ func IsValidUPX(ctx context.Context, fc []byte, path string) (bool, error) {
 		return false, err
 	}
 
-	cmd := exec.CommandContext(ctx, "upx", "-l", "-f", path)
+	base := filepath.Base(path)
+	if strings.HasPrefix(path, "-") || strings.HasPrefix(base, "-") {
+		return false, fmt.Errorf("path and/or file begins with '-': %q", path)
+	}
+	if len(base) > 255 {
+		return false, fmt.Errorf("file name exceeds 255 characters")
+	}
+
+	absPath, err := filepath.Abs(path)
+	if err != nil {
+		return false, err
+	}
+
+	cmd := exec.CommandContext(ctx, "upx", "-l", "-f", "--", absPath)
 	output, err := cmd.CombinedOutput()
 
 	if err != nil && (bytes.Contains(output, []byte("NotPackedException")) ||