[Security] The remote-media byte cap and redirect refusal have no real test #618
Labels
No labels
accepted
bug
declined
docs
duplicate
enhancement
good first issue
in-progress
needs-info
privacy
security
triage
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
LibreKAT/Ocideck#618
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Found in the pre-publication test review.
Evidence:
test/media_fetch_test.dart:53is the only assertion in the test named for the byte cap:In the coverage report,
lib/utils/media_fetch_io.dart:54(request.followRedirects = false;),:61(the Content-Length cap) and:65-70(the counting cap) have zero hits.test/network_sink_guard_test.dart:128only scans forRegExp(r"HttpClient\(")per file plus a count — the prose line at:132about "no redirects plus byte cap" is a comment, not an assertion. Delete line 54 and nothing turns red.SECURITY.md:311makes this a public promise: "redirects (a 3xx must not walk around the host check), downloads are capped".Why this matters now: the code is correct today, but on publication SECURITY.md becomes a promise to outsiders, and the first person to run
grep followRedirects test/sees that nothing stands behind it. The project already knows how to do this: for the CVE download the seam was carved out intoGithubBulkTransport.streamCappedspecifically "so the limit is reachable here" (test/cve_download_cap_test.dart:16-18).Proposal: carve the same seam in
media_fetch_io.dart— areadCapped(Stream<List<int>>, {maxBytes})and anassertNoRedirect(HttpClientResponse)— and test those two directly. No server, no socket, three assertions.