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

Open Beginner friendly
#2,056 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
csharp
Domain
security

Research direction

Start in src/code/InstallHelper.cs at InstallHelper.TryExtractToDirectory, around the directory creation and containment-check logic. Trace how entry.FullName becomes destinationPath and verify that containment is checked before any directory or file side effect. Done means entries rejected by containment leave no directories behind, while valid entries still extract normally.

Written by the indexing model from the issue text.

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.

Dominant language
C#
Stars
576
Forks
114
Avg merge
1d 2h
Merged PRs (30d)
7

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from PowerShell/PSResourceGet

All issues in PowerShell/PSResourceGet

Similar issues

More C# issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.