Skip to content

Commit 642fa56

Browse files
fix: eliminate TOCTOU race in zip packaging (#3855)
* fix: eliminate TOCTOU race in zip packaging Open file once and derive both stat info and content from the same file descriptor to prevent race conditions where the file is modified between stat() and read_bytes() calls. * test: add regression test for TOCTOU stat/read consistency in packager The old implementation called file_path.stat() then file_path.read_bytes() as separate syscalls. The fix opens the file once and uses os.fstat() + fh.read() on the same handle. This test verifies the archived bytes and mode are consistent with the opened file descriptor.
1 parent 521020b commit 642fa56

2 files changed

Lines changed: 31 additions & 3 deletions

File tree

src/specify_cli/bundler/services/packager.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,11 @@ def build_bundle(
9393
# extraction, but collapse to two canonical modes (0755 when any
9494
# execute bit is set on the source, otherwise 0644) so identical
9595
# inputs yield a byte-for-byte identical artifact.
96-
mode = 0o755 if file_path.stat().st_mode & 0o111 else 0o644
97-
info.external_attr = mode << 16
98-
archive.writestr(info, file_path.read_bytes())
96+
with file_path.open("rb") as fh:
97+
st = os.fstat(fh.fileno())
98+
mode = 0o755 if st.st_mode & 0o111 else 0o644
99+
info.external_attr = mode << 16
100+
archive.writestr(info, fh.read())
99101

100102
return BuildResult(artifact_path=artifact_path, file_count=len(files))
101103

tests/unit/test_bundler_packager.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,4 +207,30 @@ def test_executable_bit_preserved_in_artifact(tmp_path: Path):
207207
}
208208
# Executable source -> 0755; plain text files -> 0644.
209209
assert modes["scripts/hook.sh"] == 0o755
210+
211+
212+
def test_toctou_stat_read_consistency(tmp_path: Path):
213+
"""Regression: stat() and read() must use the same file descriptor.
214+
215+
The old implementation called file_path.stat() then file_path.read_bytes()
216+
as separate syscalls. Between the two, another process could replace the
217+
file. The fix opens the file once and uses os.fstat() + fh.read() on the
218+
same handle. This test verifies the archived bytes and mode are consistent.
219+
"""
220+
bundle = _make_bundle(tmp_path / "b")
221+
target = bundle / "assets" / "data.bin"
222+
target.parent.mkdir(parents=True, exist_ok=True)
223+
target.write_bytes(b"\x00\x01\x02\x03")
224+
target.chmod(0o644)
225+
226+
result = build_bundle(bundle, output_dir=tmp_path / "out")
227+
with zipfile.ZipFile(result.artifact_path) as archive:
228+
content = archive.read("assets/data.bin")
229+
modes = {
230+
info.filename: (info.external_attr >> 16) & 0o777
231+
for info in archive.infolist()
232+
}
233+
234+
assert content == b"\x00\x01\x02\x03"
235+
assert modes["assets/data.bin"] == 0o644
210236
assert modes["README.md"] == 0o644

0 commit comments

Comments
 (0)