Core Rule Set: gitea's ordinary requests pass by default (#208)

internal/waf/waf_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http/httptest"
 	"net/netip"
 	"net/url"
+	"os"
 	"path/filepath"
 	"reflect"
 	"strconv"
@@ -119,7 +120,7 @@ func atDefaults(t *testing.T) *waf.CoreRuleSet {
 }
 
 // wantChange checks that crs lets through passes, a gitea request one of
-// the six changes is for, and still finds result in refused, a request
+// the eight changes is for, and still finds result in refused, a request
 // like it that the change is not for.
 func wantChange(
 	t *testing.T, crs *waf.CoreRuleSet, passes, refused request, result waf.Result,
@@ -308,13 +309,14 @@ func TestCookiesGiteaFlashAndRedirectToAreNotRead(t *testing.T) {
 		matched(930120))
 }
 
-func TestRefererIsNotCheckedForACommandOrJavaStartingAProcess(t *testing.T) {
+func TestRefererIsNotCheckedForACommandOrJava(t *testing.T) {
 	t.Parallel()
 
 	const (
 		search      = "https://git.example/explore/repos?q=env"
 		runtimeJava = "https://git.example/openjdk/jdk/src/branch/master/src/" +
 			"java.base/share/classes/java/lang/Runtime.java"
+		stringMD = "https://git.example/owner/repo/src/branch/main/java.lang.String.md"
 	)
 
 	crs := atDefaults(t)
@@ -323,6 +325,8 @@ func TestRefererIsNotCheckedForACommandOrJavaStartingAProcess(t *testing.T) {
 		matched(932340))
 	wantChange(t, crs, get("/", "Referer: "+runtimeJava),
 		get("/", "X-Page: "+runtimeJava), matched(944110))
+	wantChange(t, crs, get("/", "Referer: "+stringMD), get("/", "X-Page: "+stringMD),
+		matched(944130))
 
 	// It is still checked for script and SQL injection.
 	wantResult(t, crs,
@@ -362,6 +366,190 @@ func TestEachDisabledRuleIsSwitchedOff(t *testing.T) {
 	wantResult(t, newCoreRuleSet(t, 1, 920350, 911100), r, waf.Result{})
 }
 
+func TestImagePushThatReusesALayerIsNotCheckedForCommandNames(t *testing.T) {
+	t.Parallel()
+
+	crs := atDefaults(t)
+	reuse := "?mount=sha256:0000&from=owner/python3-app"
+
+	wantChange(t, crs, get("/v2/owner/app/blobs/uploads/"+reuse), get("/upload/"+reuse),
+		matched(932260))
+
+	// A command after the image's name is still refused.
+	wantResult(t, crs,
+		get("/v2/owner/app/blobs/uploads/?from=owner/app;cat%20/etc/passwd"),
+		matched(930120, 932160))
+}
+
+// withExclusions returns the Core Rule Set at the defaults, with the
+// exclusions file at path.
+func withExclusions(path string) (*waf.CoreRuleSet, error) {
+	return waf.New(waf.Params{
+		ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, ExclusionsFile: path,
+	})
+}
+
+// exclusionsFile writes text to a file of exclusions and returns its path.
+func exclusionsFile(t *testing.T, text string) string {
+	t.Helper()
+
+	path := filepath.Join(t.TempDir(), "exclusions.conf")
+
+	err := os.WriteFile(path, []byte(text), 0o600)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return path
+}
+
+// backupFileExclusion is an exclusions file that switches 920500, a rule
+// of phase 1, off for the path of a backup file under /files/.
+const backupFileExclusion = `SecRule REQUEST_FILENAME "@beginsWith /files/" ` +
+	`"id:1,phase:1,pass,nolog,ctl:ruleRemoveTargetById=920500;REQUEST_FILENAME"` + "\n"
+
+func TestExclusionsFileIsLoadedBeforeTheRules(t *testing.T) {
+	t.Parallel()
+
+	crs, err := withExclusions(exclusionsFile(t, backupFileExclusion))
+	if err != nil {
+		t.Fatalf("load the exclusions: %v", err)
+	}
+
+	wantChange(t, crs, get("/files/notes.txt~"), get("/notes.txt~"), matched(920500))
+}
+
+func TestRuleSwitchedOffAndNamedInTheExclusionsFileIsOff(t *testing.T) {
+	t.Parallel()
+
+	crs, err := waf.New(waf.Params{
+		ParanoiaLevel: 1, DisabledRules: []int{920500},
+		ExclusionsFile: exclusionsFile(t, backupFileExclusion),
+	})
+	if err != nil {
+		t.Fatalf("load the exclusions: %v", err)
+	}
+
+	wantResult(t, crs, get("/notes.txt~"), waf.Result{})
+}
+
+func TestExclusionsFileMayNameAnyNumberInAComment(t *testing.T) {
+	t.Parallel()
+
+	_, err := withExclusions(exclusionsFile(t,
+		"# 942100: GET /issues/900123\n"+backupFileExclusion))
+	if err != nil {
+		t.Errorf("the exclusions did not load: %v", err)
+	}
+}
+
+func TestExclusionsFileThatCannotBeUsedIsAnError(t *testing.T) {
+	t.Parallel()
+
+	missing := filepath.Join(t.TempDir(), "missing.conf")
+
+	_, err := withExclusions(missing)
+	if err == nil {
+		t.Errorf("%s loaded", missing)
+	}
+
+	for _, tc := range []struct{ text, want string }{
+		{"SecNoSuchDirective On\n", "unknown directive"},
+		// smallwebwaf's own rules may not be switched off or changed, by
+		// id or in a range, on one line or split over several.
+		{"SecRuleRemoveById 899999-901000\n", "names 899999-901000"},
+		{
+			"SecAction \"id:1,phase:1,pass,nolog,ctl:ruleRemoveById=899999-901000\"\n",
+			"names 899999-901000",
+		},
+		{
+			"SecAction \"id:1,phase:1,pass,nolog,ctl:ruleRemoveById=899999-+901000\"\n",
+			"names 899999-+901000",
+		},
+		{
+			"SecAction \"id:1,phase:1,pass,nolog,ctl:ruleRemoveTargetById=900500;ARGS:from\"\n",
+			"names 900500",
+		},
+		{"SecRuleRemoveById 899000-\\\n    901000\n", "names 899000-901000"},
+		{
+			"SecAction \"id:1,phase:1,pass,nolog,\\\n" +
+				"    ctl:ruleRemoveById=9005\\\n    00\"\n",
+			"names 900500",
+		},
+		// The Core Rule Set's rules load after the file.
+		{"SecRuleRemoveById 942100\n", "SecRuleRemoveById in the exclusions file"},
+		{
+			"SecRuleUpdateTargetById 942100 \"!ARGS:id\"\n",
+			"SecRuleUpdateTargetById in the exclusions file",
+		},
+	} {
+		_, err := withExclusions(exclusionsFile(t, tc.text))
+		if err == nil || !strings.Contains(err.Error(), tc.want) {
+			t.Errorf("%q gave the error %v, want one with %q", tc.text, err, tc.want)
+		}
+	}
+}
+
+// giteaRequests returns the recorded ordinary gitea requests in testdata.
+// Each has the Referer of a page that names the same, as a page opened
+// from it has.
+func giteaRequests(t *testing.T) []request {
+	t.Helper()
+
+	text, err := os.ReadFile(filepath.Join("testdata", "gitea-requests.txt"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var requests []request
+
+	for line := range strings.Lines(string(text)) {
+		line = strings.TrimSpace(line)
+		if line == "" || strings.HasPrefix(line, "#") {
+			continue
+		}
+
+		target, header, _ := strings.Cut(line, " ")
+
+		headers := []string{"Referer: http://git.example" + target}
+		if header != "" {
+			headers = append(headers, header)
+		}
+
+		requests = append(requests, get(target, headers...))
+	}
+
+	return requests
+}
+
+func TestOrdinaryGiteaRequestsPassAndAttacksAreRefused(t *testing.T) {
+	t.Parallel()
+
+	const threshold = 5 // SWWAF_WAF_ANOMALY_THRESHOLD's default
+
+	crs := atDefaults(t)
+
+	for _, r := range giteaRequests(t) {
+		if got := inspect(t, crs, r); got.Score >= threshold {
+			t.Errorf("%s %q refused: %+v", r.target, r.headers[1:], got)
+		}
+	}
+
+	// In q too, off gitea's search pages.
+	for _, parameter := range []string{"id", "q"} {
+		for _, attack := range []string{
+			"1'%20OR%20'1'='1",
+			"..%2Fcustom%2Fconf%2Fapp.ini",
+			"%3Cscript%3Ealert(1)%3C%2Fscript%3E",
+		} {
+			target := "/owner/repo?" + parameter + "=" + attack
+			if got := inspect(t, crs, get(target)); got.Score < threshold {
+				t.Errorf("%s let through: %+v", target, got)
+			}
+		}
+	}
+}
+
 // bodyLimit is SWWAF_WAF_BODY_LIMIT in the tests that read bodies.
 const bodyLimit = 8 << 10