Skip to content

Commit a6770c6

Browse files
committed
fix(sync): reject --subdir that escapes --dest
dest_root = os.path.join(--dest, --subdir) let an absolute --subdir discard --dest entirely, and a --subdir containing .. walk out of the dest tree; copy mode then shutil.copy2-overwrote files there. Require --subdir to be relative and to resolve to a path inside --dest. Adds tests/test_wolfglass_sync.py (absolute and .. rejected, relative copies inside) and wires it into selftest.yml.
1 parent bd02c67 commit a6770c6

3 files changed

Lines changed: 74 additions & 1 deletion

File tree

.github/workflows/selftest.yml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,14 @@ jobs:
3333
provenance/bomsh_verify.py \
3434
tools/wolfglass-sync \
3535
tests/test_gen_sbom.py \
36-
tests/test_sbom.py
36+
tests/test_sbom.py \
37+
tests/test_wolfglass_sync.py
3738
3839
- name: Run generator unit tests
3940
run: python -m unittest tests/test_gen_sbom.py
4041

42+
- name: Run wolfglass-sync tests
43+
run: python -m unittest tests/test_wolfglass_sync.py
44+
4145
- name: Run self-test
4246
run: python tests/test_sbom.py

tests/test_wolfglass_sync.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
#!/usr/bin/env python3
2+
"""Tests for tools/wolfglass-sync, focused on the --subdir containment guard.
3+
4+
--subdir is joined onto --dest and then written into, so an absolute value or
5+
one containing .. must be refused rather than allowed to place/overwrite files
6+
outside the product's vendoring path."""
7+
8+
import os
9+
import subprocess
10+
import sys
11+
import tempfile
12+
import unittest
13+
14+
REPO = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
15+
SYNC = os.path.join(REPO, "tools", "wolfglass-sync")
16+
17+
18+
def _run(dest, subdir):
19+
return subprocess.run(
20+
[sys.executable, SYNC, "--dest", dest, "--subdir", subdir],
21+
capture_output=True, text=True)
22+
23+
24+
class TestSubdirContainment(unittest.TestCase):
25+
def test_relative_subdir_copies_inside_dest(self):
26+
with tempfile.TemporaryDirectory() as dest:
27+
r = _run(dest, "tools/sbom")
28+
self.assertEqual(r.returncode, 0, r.stderr)
29+
# gen-sbom is part of share/, so it must land under the subdir.
30+
self.assertTrue(
31+
os.path.isfile(os.path.join(dest, "tools", "sbom", "gen-sbom")),
32+
os.listdir(dest))
33+
34+
def test_absolute_subdir_rejected(self):
35+
with tempfile.TemporaryDirectory() as dest, \
36+
tempfile.TemporaryDirectory() as outside:
37+
target = os.path.join(outside, "pwned")
38+
r = _run(dest, target) # absolute path
39+
self.assertNotEqual(r.returncode, 0)
40+
self.assertIn("--subdir", r.stderr)
41+
self.assertFalse(os.path.exists(target),
42+
"absolute --subdir wrote outside --dest")
43+
44+
def test_dotdot_subdir_rejected(self):
45+
with tempfile.TemporaryDirectory() as parent:
46+
dest = os.path.join(parent, "product")
47+
os.mkdir(dest)
48+
r = _run(dest, "../escape")
49+
self.assertNotEqual(r.returncode, 0)
50+
self.assertIn("escapes --dest", r.stderr)
51+
self.assertFalse(os.path.exists(os.path.join(parent, "escape")),
52+
"../ --subdir wrote outside --dest")
53+
54+
55+
if __name__ == "__main__":
56+
unittest.main(verbosity=2)

tools/wolfglass-sync

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,20 @@ def main():
5050
if not os.path.isdir(share_dir):
5151
sys.exit(f"ERROR: no share/ directory at {share_dir}")
5252

53+
# --subdir is joined onto --dest and then written into. An absolute value
54+
# (os.path.join discards --dest) or one containing .. would place and
55+
# overwrite files outside the product's vendoring path, so require it to
56+
# resolve to a location inside --dest.
57+
if os.path.isabs(args.subdir):
58+
sys.exit(f"ERROR: --subdir must be relative to --dest, got absolute "
59+
f"{args.subdir!r}.")
5360
dest_root = os.path.join(args.dest, args.subdir)
61+
dest_abs = os.path.realpath(args.dest)
62+
dest_root_abs = os.path.realpath(dest_root)
63+
if dest_root_abs != dest_abs and \
64+
not dest_root_abs.startswith(dest_abs + os.sep):
65+
sys.exit(f"ERROR: --subdir {args.subdir!r} escapes --dest; it must "
66+
f"resolve to a path inside {args.dest!r}.")
5467
files = sorted(iter_share_files(share_dir), key=lambda p: p[1])
5568

5669
if args.check:

0 commit comments

Comments
 (0)