Path-based package dependencies can escape the parent package root

Classification

Type: unsafe trust-boundary violation
Severity: high
Confidence: certain

Affected Locations

src/Package/Fetch.zig:526
src/Package/Fetch.zig:963
src/main.zig:5343
lib/std/Build/Cache/Path.zig:40

Summary

Recursive package fetching accepts dependencies.<name>.path values from a fetched package's manifest and resolves them with .. segments, but the containment check is tied to the wrong root directory and therefore does not stop the resolved path from escaping the parent package directory. A fetched package can make Zig read arbitrary files and directories reachable from the build root instead of remaining confined to its own package contents.

Preconditions

A build uses recursive package fetching (src/main.zig:5343 sets recursive = true for zig build).
A fetched package contains a build.zig.zon manifest with a path dependency (src/Package/Fetch.zig:891, src/Package/Fetch.zig:960).
The attacker controls that fetched package's manifest contents.

Proof

Input/source/state origin queueJobsForDeps iterates over manifest dependencies from the fetched package and handles .path entries by taking the manifest-controlled rel_path string (src/Package/Fetch.zig:891, src/Package/Fetch.zig:960).
Control-flow and data-flow path The path is resolved with f.package_root.resolvePosix(parent_arena, rel_path) (src/Package/Fetch.zig:963). resolvePosix preserves leading .. components when they escape the base path, yielding paths outside the package root while keeping the same root directory handle (lib/std/Build/Cache/Path.zig:40, lib/std/fs/path.zig:1107). The resulting Cache.Path is stored as a new fetch job's .relative_path location (src/Package/Fetch.zig:975).
Failing condition or violated invariant When that fetch job runs, the only escape check is inside if (pkg_root.root_dir.eql(local_cache_root.root_dir)) (src/Package/Fetch.zig:537). During recursive builds, local_cache_root is .zig-cache (src/main.zig:5343), while fetched packages and their path dependencies live under root_pkg_path = build_root.directory + "zig-pkg" (src/main.zig:5344). Because these root directory handles differ, the guard is false and no containment check runs. The code then accepts pkg_root and continues with loadManifest / checkBuildFileExistence / queueJobsForDeps (src/Package/Fetch.zig:563-567).
Resulting impact Subsequent operations open files relative to the escaped pkg_root, for example loadManifest reads build.zig.zon from that path and initResource opens file: URLs relative to f.parent_package_root with no additional containment enforcement (src/Package/Fetch.zig:563, src/Package/Fetch.zig:1187). Therefore a fetched package can cause Zig to consume manifests and package files from arbitrary sibling or ancestor paths reachable from the build root instead of only from its own unpacked contents.
Why this is reachable in the current code The recursive fetch path is the normal zig build package-resolution path (src/main.zig:5328-5412). The resolver explicitly allows .path dependencies from manifests and expects invalid paths to be checked only at the start of run() (src/Package/Fetch.zig:961-963). In the current configuration, that check does not run for the actual package root used by fetched dependencies because it compares against .zig-cache, not zig-pkg.

Why This Is A Real Bug

This is a concrete containment failure in reachable production code. The manifest-controlled path is resolved, accepted, and dereferenced by the current implementation. No environment-specific assumption is needed beyond invoking normal recursive dependency resolution. The bug is not stylistic: the code intends to reject dependencies "outside project" (src/Package/Fetch.zig:559) but fails to do so for the actual fetched-package storage root used in recursive builds.

Fix Requirement

A code change is required because the current guard checks the wrong directory identity and does not enforce that a non-root path dependency stays within the parent package root. Without a fix, .. traversal in fetched manifests remains effective.

Patch Rationale

The patch replaces the root-directory-specific check with a direct descendant check against f.parent_package_root for recursive path dependencies. It accepts the parent directory itself and any separator-delimited child path, while rejecting sibling-prefix tricks such as hash_evil when the parent is hash. This is the minimal local fix because it changes only the containment decision and preserves all other fetch behavior.

Residual Risk

Patch

Reference: patch_001.diff

Credits

Found by Swival security scanner.

diff --git a/src/Package/Fetch.zig b/src/Package/Fetch.zig --- a/src/Package/Fetch.zig +++ b/src/Package/Fetch.zig @@ -532,29 +532,31 @@ pub fn run(f: *Fetch) RunError!void { hash_tok, try eb.addString("path-based dependencies are not hashed"), );

       // Packages fetched by URL may not use relative paths to escape outside the

       // fetched package directory from within the package cache.

       if (pkg_root.root_dir.eql(local_cache_root.root_dir)) {

           // `parent_package_root.sub_path` contains a path like this:

```
           // "p/$hash", or
```

           // "p/$hash/foo", with possibly more directories after "foo".

           // We want to fail unless the resolved relative path has a

```
           // prefix of "p/$hash/".
```

           const prefix_len: usize = if (job_queue.read_only) 0 else "p/".len;

           const parent_sub_path = f.parent_package_root.sub_path;

```
           const end = find_end: {
```

               if (parent_sub_path.len > prefix_len) {

                   // Use `isSep` instead of `indexOfScalarPos` to account for

                   // Windows accepting both `\\` and `/` as path separators.

                   for (parent_sub_path[prefix_len..], prefix_len..) |c, i| {

                       if (std.fs.path.isSep(c)) break :find_end i;

```
                   }
```

       // Packages fetched by URL may not use relative paths to escape outside the

       // fetched package directory from within the package cache.

       if (f.parent_manifest_ast != null and pkg_root.root_dir.eql(f.parent_package_root.root_dir)) {

           const parent_sub_path = f.parent_package_root.sub_path;

           const within_parent = std.mem.eql(u8, pkg_root.sub_path, parent_sub_path) or blk: {

               if (!std.mem.startsWith(u8, pkg_root.sub_path, parent_sub_path)) break :blk false;

               if (parent_sub_path.len == 0) break :blk true;

               if (pkg_root.sub_path.len == parent_sub_path.len) break :blk true;

               break :blk std.fs.path.isSep(pkg_root.sub_path[parent_sub_path.len]);

```
           };
```
```
           if (!within_parent) {
```
```
               return f.fail(
```
```
                   f.location_tok,
```

                   try eb.printString("dependency path outside project: '{f}'", .{pkg_root}),

```
               );
```
```
           }
```

       } else if (pkg_root.root_dir.eql(local_cache_root.root_dir)) {

           // `parent_package_root.sub_path` contains a path like this:

```
           // "p/$hash", or
```

           // "p/$hash/foo", with possibly more directories after "foo".

           // We want to fail unless the resolved relative path has a

```
           // prefix of "p/$hash/".
```

           const prefix_len: usize = if (job_queue.read_only) 0 else "p/".len;

           const parent_sub_path = f.parent_package_root.sub_path;

```
           const end = find_end: {
```

               if (parent_sub_path.len > prefix_len) {

                   // Use `isSep` instead of `indexOfScalarPos` to account for

                   // Windows accepting both `\\` and `/` as path separators.

                   for (parent_sub_path[prefix_len..], prefix_len..) |c, i| {

                       if (std.fs.path.isSep(c)) break :find_end i;

```
                   }
               }
```

               break :find_end parent_sub_path.len;

```
           };
```

           const expected_prefix = parent_sub_path[0..end];

           if (!std.mem.startsWith(u8, pkg_root.sub_path, expected_prefix)) {

```
               return f.fail(
```
```
                   f.location_tok,
```

                   try eb.printString("dependency path outside project: '{f}'", .{pkg_root}),

```
               );
```

               break :find_end parent_sub_path.len;

```
           };
```

           const expected_prefix = parent_sub_path[0..end];

           if (!std.mem.startsWith(u8, pkg_root.sub_path, expected_prefix)) {

```
               return f.fail(
```
```
                   f.location_tok,
```

                   try eb.printString("dependency path outside project: '{f}'", .{pkg_root}),

               );
           }
       }
       f.package_root = pkg_root;

jedisct1/report_001.md

Select an option

No results found