Skip to content

Create parent directories only after the containment check in InstallHelper.TryExtractToDirectory #2056

Description

Summary of the new feature / enhancement

During package extraction, entry-derived parent directories are created before the path-containment decision runs. An entry whose file write the containment check would reject can still leave empty directories outside the extraction root (directory-only effect — no file content is written outside). As a user, I want each entry's extraction side effects to be all-or-nothing, so a malformed package cannot leave stray directories anywhere on disk.

Proposed technical implementation details

In src/code/InstallHelper.cs, TryExtractToDirectory (~L1299 on current main):

  • For entries containing a path separator, Directory.CreateDirectory(Path.Combine(extractPath, parentDirs)) runs first (~L1372-1377), using the raw entry prefix.
  • Only afterwards is destinationPath = Path.GetFullPath(Path.Combine(extractPath, entry.FullName)) computed and the StartsWith(extractPath) containment verified (~L1381-1386) — and that check gates only the ExtractToFile call (~L1388).

So a package containing an entry such as ../../../../empty/dir/x (this loop is reached on installs from HTTP repositories, which do not go through a BCL extraction up front) creates empty/dir relative to the extraction temp directory as empty directories, and then the file write is skipped by the containment check.

Suggested reordering, so containment gates every side effect:

  1. Compute destinationPath = Path.GetFullPath(Path.Combine(extractPath, entry.FullName)) first.
  2. Verify containment of destinationPath under extractPath.
  3. Only then create Path.GetDirectoryName(destinationPath) and call ExtractToFile.
  4. Skip entries that fail containment entirely — no partial side effects.

This also makes directory creation consistent with the resolved path rather than the raw entry string.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions