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.

Fuzz the parsers that read untrusted input

16bf0ebf8054de5044a52f93d156402542f0f71a · Keenan Nemetz · 10d ago · parent 70ba9a69

Fuzz the parsers that read untrusted input

The SSH exec line, the Git HTTP route, and the LFS packet stream are all
hand-written parsers on bytes a client controls, and each was covered only by a
table of examples someone thought to write.

Six targets, each asserting a property rather than waiting for a panic: the
command parser cannot invent bytes and its output survives being quoted back, a
route re-routes from its own parts and never names more than one path segment, a
repository argument is a name and not a path, and the packet reader's lines
survive being framed again. The seeds are the shapes the clients actually send,
so go test ./... keeps guarding them and a fuzz target costs a push nothing.

The route fuzzer found one gap on its first run: /git/.git/info/refs gave back an
empty repository name with ok set, and only the store refused it. A route that
answers ok now means a name the rest of the path applies to.

Closes #96.

server/githttp/fuzz_test.go +82

@@ -0,0 +1,82 @@
1 package githttp
2
3 import (
4 "strings"
5 "testing"
6 )
7
8 // FuzzRoute drives the router that turns an anonymous request path into a
9 // repository name and the rest of the Git service path.
10 //
11 // The properties matter because the name is used as a name, not as a path: it is
12 // handed to the repository store, which must not be asked for a path that names
13 // two repositories or escapes upward. Route stops at the first slash, so the
14 // guarantee is structural, and the fuzzer's job is to show that it holds for
15 // every input rather than for the ones in the table.
16 func FuzzRoute(f *testing.F) {
17 for _, seed := range []string{
18 "/git/demo.git/info/refs",
19 "/git/demo.git/git-upload-pack",
20 "/git/demo.git/git-receive-pack",
21 "/git/demo.git/",
22 "/git/demo.git",
23 "/git/.git/info/refs",
24 "/git/demo/info/refs",
25 "/git/demo.git/%2e%2e/info/refs",
26 "/git/../demo.git/info/refs",
27 "/git/a.git//info/refs",
28 "git/demo.git/info/refs",
29 "",
30 } {
31 f.Add(seed)
32 }
33 f.Fuzz(func(t *testing.T, path string) {
34 name, rest, ok := Route(path)
35 if !ok {
36 return
37 }
38 if name == "" {
39 t.Fatalf("Route(%q) accepted an empty repository name", path)
40 }
41 if strings.Contains(name, "/") {
42 t.Fatalf("Route(%q) returned name %q, which names more than one path segment", path, name)
43 }
44 // The same request has to route the same way when it is written back out
45 // from its own parts. That is what makes the split a route rather than a
46 // prefix that happens to match once.
47 name2, rest2, ok2 := Route("/git/" + name + ".git/" + rest)
48 if !ok2 || name2 != name || rest2 != rest {
49 t.Fatalf("Route(%q) = (%q, %q), but routing from those parts gives (%q, %q, %v)", path, name, rest, name2, rest2, ok2)
50 }
51 })
52 }
53
54 // FuzzIsPush drives the decision that separates a read from a write, which is the
55 // one call that decides whether a request is offered the receive path at all.
56 func FuzzIsPush(f *testing.F) {
57 for _, seed := range []struct{ rest, query string }{
58 {"info/refs", "service=git-receive-pack"},
59 {"info/refs", "service=git-upload-pack"},
60 {"info/refs", ""},
61 {"info/refs", "service=git-receive-pack&version=1"},
62 {"info/refs", "service=git-upload-pack&service=git-receive-pack"},
63 {"git-receive-pack", ""},
64 {"git-upload-pack", ""},
65 {"", ""},
66 } {
67 f.Add(seed.rest, seed.query)
68 }
69 f.Fuzz(func(t *testing.T, rest, query string) {
70 push := IsPush(rest, query)
71 // A pair that names no service cannot change the answer, which is what makes
72 // the query parsing safe to extend.
73 if pushed := IsPush(rest, query+"&version=1"); pushed != push {
74 t.Fatalf("IsPush(%q, %q) = %v, but adding an unrelated pair gives %v", rest, query, push, pushed)
75 }
76 // A receive-pack path is a write whatever the query says; the query only
77 // matters for the advertisement.
78 if strings.HasPrefix(rest, "git-receive-pack") && !push {
79 t.Fatalf("IsPush(%q, %q) = false for a receive-pack path", rest, query)
80 }
81 })
82 }