fix: reject path traversal in the post id route - #28
Merged
Conversation
view_post interpolated the URL path parameter straight into
`content/{post_id}.md` with no validation. Rocket percent-decodes
dynamic path segments before the handler sees them, so an encoded
`..%2f` escaped the content directory and let any `.md` file on the
host be read over HTTP. The rendered branch and `GET /nojs/<post_id>`
were affected too, since both reach the same handler.
Add `is_valid_post_id`, which accepts only the `[A-Za-z0-9_-]` slugs
the application actually produces, and reject anything else at the
request boundary before any filesystem access. Because `.`, `/`, and
`\` are unrepresentable, a traversal sequence can no longer be formed.
Add unit tests covering accepted slugs, rejected traversal vectors,
the length bound, and the invariant that every generated id passes the
guard.
Closes du82#27
Owner
|
Will pay bounty to address in this comment: #31 (comment) |
Zhiyilang074811
added a commit
to Zhiyilang074811/nonograph
that referenced
this pull request
Sep 7, 2026
Add is_valid_post_id() to validate post_id at the trust boundary before any filesystem access. Only allows [a-z0-9_-] characters, matching the exact charset that generate_post_id produces. Closes du82#28 Refs: CWE-22, OWASP Path Traversal
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
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.
Closes #27
What
GET /<post_id>(and, by delegation,GET /nojs/<post_id>) interpolated theURL path parameter straight into
content/{post_id}.mdwith no validation.Rocket percent-decodes dynamic path segments before the handler runs, so an
encoded
..%2fescaped thecontent/directory and let any.mdfile on thehost be read over HTTP. Confirmed against a proxy-less instance; a
path-normalizing reverse proxy (e.g. Caddy in front of
nonogra.ph) happens tomitigate it, but a directly exposed Rocket -- including the onion service in the
shipped
Dockerfile-- is fully affected.How
is_valid_post_id, which accepts only the[A-Za-z0-9_-]slugs theapplication already produces (
generate_post_idemits[a-z0-9-], the staticpages are lowercase words, and the Telegraph archiver yields
[A-Za-z0-9_-]). Because.,/, and\are unrepresentable, a traversalsequence can no longer be formed.
view_post, before any filesystemaccess, returning the normal 404 for anything invalid.
The check is applied once, at the trust boundary between untrusted path input
and the filesystem, rather than scattered across call sites.
Tests
Added unit tests covering:
..,../README,..\README,foo/bar,foo.bar,post.md, embedded NUL, empty);generate_post_idcan emit passes the guard, sono freshly created post 404s.
Full suite:
cargo test-> 144 passed. Manually verified against a runninginstance that traversal now returns 404 on all vectors (including
/nojs),while normal pages, raw
.mdreads, and a round-tripped published post stillreturn 200.
References