Audit round two: four more ways the tool could be turned on its user - #35
Merged
Conversation
**`keygen` printed "(mode 600)" over a world-readable key.** `os.open`'s mode argument applies only when the file is created, so a key file left by an earlier run under a wider umask — or a placeholder somebody touched — kept its permissions through `O_TRUNC` while the CLI reported 600. A claim printed next to a fact that contradicts it is the defect class this project exists to find elsewhere. `fchmod` on the open descriptor closes it without a window in which the path could be swapped. **The media probe fetched any URL the response body named.** The endpoint under test supplies asset URLs, so it chose what markproof requested — and pointed at 169.254.169.254, which on every major cloud answers with instance credentials, that is a server-side request forgery made by a trusted process from inside the operator's network. Loopback, private, link-local and reserved ranges are now refused, but only when the probe's own target is not itself internal: pointing markproof at a bot on localhost is the documented way to try it, and breaking that would break the demo the README sends people to. Literal addresses only. Resolving hostnames here would make a verdict depend on DNS at check time, which is exactly what the determinism claim rules out. **`patterns_file` and `labels_file` could leave the packaged directory.** A rulepack is loaded from a path the operator passes, so its contents are only as trusted as that file — and the loader resolved these names against the package directory without checking they were names. Both are now validated as bare filenames. **The manifest fetch had no size limit and loose status handling.** The document had a limit; advertising a manifest was the cheapest way around it. Non-200 is now refused rather than only 4xx and 5xx: a 204 carries no manifest either, and handing the checker an empty body while calling it a manifest is the wrong kind of wrong. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows #34. The remaining security-shaped findings from the launch audit.
keygenprinted a permission it had not set…over a file that was
-rw-r--r--.os.open's mode argument applies only oncreation, so a key file left by an earlier run under a wider umask, or a
placeholder somebody touched, keeps its permissions through
O_TRUNC.I checked this one before believing it — a fresh
keygenreally does produce 600,and the auditor's claim looked wrong until I tried the pre-existing-file case.
fchmodon the open descriptor closes it with no window in which the path couldbe swapped.
A claim printed next to a fact that contradicts it is precisely the defect class
this project exists to find in other people's systems.
The response body chose what markproof fetched
The media probe fetches asset URLs out of the endpoint's own JSON. So the endpoint
under test decided what a trusted process requested from inside the operator's
network — and
http://169.254.169.254/latest/meta-data/answers with instancecredentials on every major cloud.
Loopback, private, link-local, reserved, multicast and unspecified addresses are
refused when the probe's own target is not itself internal. That condition is
the interesting part: pointing markproof at the demo bot on
127.0.0.1is thedocumented way to try it, and an absolute rule would have broken the first thing
the README asks a newcomer to do.
Literal addresses only — resolving hostnames would make a verdict depend on DNS at
check time, which is what the determinism claim rules out. That defence belongs in
the network the runner sits on.
Two smaller ones
patterns_file/labels_filecould leave the packaged directory. A rulepackcomes from a path the operator passes, so it is only as trusted as that file, and
the loader resolved these against the package directory without checking they were
names. Now validated as bare filenames.
The manifest fetch was unbounded. The document had a size limit; advertising a
manifest was the cheapest way around it. Non-200 is now refused rather than only
4xx/5xx — a 204 carries no manifest either, and handing the checker an empty body
while calling it a manifest is the wrong kind of wrong.
Verification
Each fix has a test that fails without it, including the parameterised SSRF cases
(
169.254.169.254,127.0.0.1,10.0.0.5,[::1]) and the "a local target maystill serve local assets" case that keeps the demo working.
522 tests, ruff + mypy strict clean.
Docs and README findings follow.
🤖 Generated with Claude Code