[Bug] SDK archive extraction does not constrain entry paths to destination

[xy200303]

Summary

The SDK-based archive extraction path constructs output paths with filepath.Join(destination, entryName) without validating that the normalized result remains inside the requested destination directory.

Affected revision

master at b47bba48273dcd56d0cfb1627632c1552ae97ba0.

Code locations

• agent/utils/files/file_op.go:1054-1089 (decompressWithSDK)
• agent/utils/files/file_op.go:1231-1257 (tryDecompressTarGz)

Expected behavior

Every archive entry should be rejected unless its resolved extraction path is inside the caller-provided destination directory.

Actual behavior

Archive member names are joined directly to the destination path. Parent-directory path components are not rejected before files or directories are created.

Suggested fix

Before creating a file or directory, resolve the candidate path and verify it is within the extraction root (for example, with filepath.Rel). Reject absolute paths, paths resolving outside the root, and unsafe link entries. Apply the same guard to every SDK extraction path.

Notes

This is submitted as a public robustness bug report with no exploit payload or deployment-specific impact details. Please use the repository security contact for any request for additional reproduction information.

Detailed technical analysis

Extraction paths affected

FileOp.Decompress chooses the SDK extractor for formats that do not take the shell-extractor branch. decompressWithSDK handles the generic SDK path, and tryDecompressTarGz is attempted for gzip archives. Both routines derive the output path by joining the destination directory and the archive entry name, then immediately create parent directories and truncate/create the output file.

The current code does not establish or enforce the invariant that a normalized candidate output path must remain under the normalized extraction root. This is especially important because archive entry names are data supplied by the archive, not trusted application paths.

Lifecycle observation

The file-management service extracts into a temporary directory before copying normal extracted files to the requested destination. This is a useful isolation step, but it is not sufficient if an archive entry resolves outside that temporary directory: cleanup removes only the temporary directory itself. The extraction layer must enforce containment before any filesystem operation.

Recommended implementation approach

1. Normalize the extraction root once using filepath.Abs and filepath.Clean.
2. For every archive member, reject an absolute member name.
3. Construct the candidate path, normalize it, and compute filepath.Rel(root, candidate).
4. Reject when the relative result is .., begins with .. plus a path separator, or is otherwise outside the root.
5. Apply the same validation before directory creation, file creation, permission changes, and timestamp changes.
6. Define an explicit policy for symbolic links, hard links, device entries, and special files. The safest default is to reject them for panel-managed extraction.

A shared helper should be used by both SDK extraction implementations so future format-specific code cannot omit the check.

Suggested regression tests

Add table-driven tests for both decompressWithSDK and tryDecompressTarGz covering:

• a normal nested relative filename, which must extract successfully;
• a member name containing parent-directory components, which must be rejected with no output outside the extraction root;
• an absolute member name, which must be rejected;
• malformed or empty entry names;
• link and special-file entries according to the selected policy;
• cleanup behavior after a rejected member.

The tests should assert both the returned error and the absence of files outside the supplied temporary extraction directory.

Scope

This report intentionally focuses on the containment invariant in the extraction utility. Callers such as file-management and application-resource installation inherit the behavior of this utility and do not need to reimplement the path check.