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/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

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

Fuzz the path validators and the Markdown sanitizer

5fce878d41da32448803f3a4f4d42c7841fba331 · Keenan Nemetz · 10d ago · parent d9f5a930

Fuzz the path validators and the Markdown sanitizer

Two pieces of untrusted text decide where a file is written and what a browser
runs: the validators that turn a request path, an artifact glob, or a site
directory into a place under a root, and the markdown boundary where goldmark and
bluemonday turn a pushed body into HTML a page trusts.

The sanitizer assertions parse the output with an HTML parser rather than
searching the string, because the difference is the whole question: a script tag
rendered as text is a text node, and hand-written <script> looks the same to a
substring search. The test asks what the browser will do: no script-shaped
element, no on* attribute, and no scheme outside http, https, mailto.

The path targets state the guarantee the way the caller uses it -- joined to a
root, the result stays under that root -- and pin that cleaning is idempotent, so
one page cannot become two cache keys.

cleanPath's comment claimed dot segments were refused; they are resolved, and the
comment now says so, which is what the property pins.

Closes #98.

server/pages/fuzz_test.go +75

@@ -0,0 +1,75 @@
1 package pages
2
3 import (
4 "path"
5 "strings"
6 "testing"
7 )
8
9 // FuzzCleanPath drives the function that turns a request path into a path inside a
10 // published tree.
11 //
12 // What it must never do is hand back a path that climbs. The tree lookup itself
13 // cannot escape — a tree is a map of names, not a filesystem — but the result is
14 // also used to build links, cache keys, and the path in a log line, and a value
15 // with `..` in it is wrong in all three.
16 //
17 // dot segments are resolved rather than refused (`../../etc/passwd` becomes
18 // `etc/passwd`), which is why the properties are about the result and not about
19 // rejecting the input.
20 func FuzzCleanPath(f *testing.F) {
21 for _, seed := range []string{
22 "index.html",
23 "docs/getting-started.md",
24 "a/b/c/d.txt",
25 "../../etc/passwd",
26 "a/../../b",
27 "./a/./b",
28 "a//b",
29 "/leading/slash",
30 "trailing/slash/",
31 "%2e%2e%2fetc%2fpasswd",
32 "a\x00b",
33 `a\b`,
34 "..",
35 ".",
36 "/",
37 "",
38 "a/b/../../../c",
39 "....//....//x",
40 } {
41 f.Add(seed)
42 }
43 f.Fuzz(func(t *testing.T, requestPath string) {
44 treePath, ok := cleanPath(requestPath)
45 if !ok {
46 return
47 }
48 if strings.HasPrefix(treePath, "/") || strings.HasSuffix(treePath, "/") {
49 t.Fatalf("cleanPath(%q) = %q, which is not a relative tree path", requestPath, treePath)
50 }
51 if strings.ContainsAny(treePath, "\\\x00") {
52 t.Fatalf("cleanPath(%q) = %q, which holds a separator the tree lookup cannot take", requestPath, treePath)
53 }
54 for _, segment := range strings.Split(treePath, "/") {
55 if treePath == "" {
56 break
57 }
58 if segment == "" || segment == "." || segment == ".." {
59 t.Fatalf("cleanPath(%q) = %q, which holds the dot segment %q", requestPath, treePath, segment)
60 }
61 }
62 // The result joined to a root stays under that root. This is the property
63 // the segment check exists to guarantee, stated the way the caller uses it.
64 joined := path.Join("/siteroot", treePath)
65 if joined != "/siteroot" && !strings.HasPrefix(joined, "/siteroot/") {
66 t.Fatalf("cleanPath(%q) = %q, which resolves to %q", requestPath, treePath, joined)
67 }
68 // Cleaning a cleaned path must not move it, or two requests for the same
69 // page would produce two cache keys.
70 again, ok := cleanPath(treePath)
71 if !ok || again != treePath {
72 t.Fatalf("cleanPath(%q) = %q, but cleaning that gives %q, %v", requestPath, treePath, again, ok)
73 }
74 })
75 }