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.

#109

Fuzz the parsers that read untrusted input

merged · by keenan 10d ago · merged into main at 16bf0ebf by keenan 10d ago

keenan opened this 10d ago

Closes #96.

Why

Three hand-written parsers run on bytes a client controls, and each was covered only by a table of examples someone thought to write:

  • splitCommand turns an exec line from an unauthenticated SSH connection into an argv — the argv that starts a git process.
  • Route in server/githttp turns an anonymous request path into a repository name and the rest of the Git service path.
  • The pkt-line reader in core/lfs/transfer.go turns a pusher’s stream into lengths and payloads.

What changed

Six fuzz targets, in the package beside the code so the unexported functions are reachable. Each asserts a property rather than settling for “no panic”:

Target Property
FuzzSplitCommand Cannot invent bytes (every argument byte came from the line), and its output survives being quoted back the way git quotes
FuzzRepoName An accepted value is a name, not a path: no separators, no .., passes the store’s own validation, and "/"+name+".git" gives the name back
FuzzRoute A name never names more than one path segment, and a request re-routes identically from its own parts
FuzzIsPush Adding a query pair that names no service cannot change the read/write answer; a receive-pack path is always a write
FuzzPktReader Cannot return more payload than the stream holds; a control packet carries nothing; lines survive being framed again
FuzzReadBatch The loops above the reader terminate on the input given and return no more than it contains

FuzzRepoName is the one target past the three the issue names. It is the next function on the same untrusted text — the SSH argument becomes a repository name — and fuzzing the parser without the step that touches storage would leave the decision that matters untested.

The route fuzzer found a gap on its first run

Route("/git/.git/info/refs") returned an empty repository name with ok set. Store.Open refuses an empty name, so the request became a 404 rather than anything worse — but the router’s contract said ok for something that can never exist, and the next caller has to remember to check. Route now refuses it, the input is in the FuzzRoute seed corpus, and it is a case in TestRoute.

Verification

  • Six targets, 60 seconds each, all clean: 51.3 M executions total (FuzzSplitCommand 8.3 M, FuzzRepoName 8.2 M, FuzzRoute 11.4 M, FuzzIsPush 8.9 M, FuzzPktReader 6.2 M, FuzzReadBatch 8.9 M).
  • vole workflow run verify in the pinned image: green, seeds included.
  • Seeds cost a push nothing: they run as ordinary subtests under go test ./..., so a fuzz target that has already found something keeps guarding it.
  • CONTRIBUTING.md gains the command and the budget rule, plus where Go writes a failing input and why it must be committed.

What this does not do

No fuzz budget runs in CI. Each target’s seeds do, which is what makes a finding permanent; spending longer is a local command. Adding a scheduled or timed fuzz job needs a trigger Vole does not have — workflows start on a push to a branch or a tag, and there is no schedule — so it belongs with #102/#104 rather than invented here.

Discussion (0)

No discussion yet.