Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

#98

P2: Fuzz the path validators and the Markdown sanitizer

closed · opened by keenan 10d ago

keenan opened this 10d ago

Why

Two pieces of untrusted text decide where a file is written and what a browser executes:

  • Path validators: cleanPath in server/pages/pages.go, and the artifact, pages, and LFS path rules under ci and server.
  • Markdown rendering and sanitization: server/web/markdown.go with goldmark and bluemonday, which sits between a comment body and every visitor.

A path that escapes its root writes outside the intended directory; a sanitizer gap is stored XSS in a trusted origin. Both are string handling, which fuzzing finds faster than reading does.

Scope

  • Fuzz the path validators: .. segments, absolute paths, backslashes and Windows separators, NULs, empty components, over-long components, encoded traversal, and Unicode that normalizes into a separator. Assert an accepted path stays under the root it was resolved against.
  • Fuzz Markdown rendering with a corpus of sanitizer-bypass shapes: javascript: and data: URLs, event-handler attributes, <script> in raw HTML, nested entities, and the autolink and table extensions the renderer enables.
  • Assert the output property directly: no <script>, no on*= attribute, and no URL scheme outside the allowed set.
  • Seed from the existing tests and document the local fuzz budget.

Acceptance

  • go test ./... runs each target’s seed corpus.
  • Each target survives a documented one-minute run without a finding.
  • The sanitizer assertions fail if bluemonday’s policy is loosened, so the target also guards the policy.

Part of #14.

Comments (1)

keenan 10d ago

Correction: there is no conflict, and this comment was wrong.

I predicted one because #110 rewrites the same paragraph of CONTRIBUTING.md that this change rewrites. What I did not notice is that I created this branch while still on #110’s branch, so 5fce878 sits directly on top of d9f5a93:

5fce878 Fuzz the path validators and the Markdown sanitizer
d9f5a93 Fuzz the metadata and workflow parsers   (now main)
16bf0eb Fuzz the parsers that read untrusted input

A textual conflict needs two branches changing the same lines from the same base. This branch already contained the other change, so there was nothing to reconcile — and the forge was right to report a clean fast-forward.

The accident had one consequence worth recording: when this change request was opened, main was still 16bf0eb, so its head included #110’s commit as well and the review page showed two issues’ worth of work. That is gone now that #110 has merged, and the diff against main is the six files above. Had #110 not merged first, landing this one would have carried #97’s commits with it.