Core Rule Set: the package registries' searches of gitea and Forgejo pass (closes #354)

Recorded from gitea 1.27.3 and Forgejo 17.0.1 with each registry's own
client, and curl for the Chef cookbook list, NuGet's Packages() and the
counts at Search()/$count and Packages()/$count, with python3,
powershell, ls -la, .htaccess, file names and plain words. q on the
Cargo, Chef, Composer and Conan searches and NuGet's search service
joins 900603. 900605 lets /bin/sh through on Composer and Conan. New:
900609 for npm's text; 900611, in phase 2, lets one q shaped as a Conan
reference with no */, such as zlib/*, skip 942100; 900612 for NuGet's
Search(), Packages() and their counts: searchTerm, $filter, and the
names $filter, $orderby and $top for 942290.

Model: opus-5-5

SPEC.md

@@ -923,8 +923,8 @@ one entry of `apps` in `internal/coreruleset/coreruleset.go`. gitea's fixes are:
         - `text` of the npm registry's search, for the same four rules and the
           rule for shell paths (932160). An npm name can hold a slash, as
           `@alice/tools-cli` does.
-        - `searchTerm` of the NuGet v2 API's searches, for the same four rules.
+        - `searchTerm` of the NuGet registry's older searches, for the same four
-          These are the searches:
+          rules. These are the searches:
             - `Search()` and `Packages()`;
             - `Search()/$count` and `Packages()/$count`, which count their
               results.
@@ -936,13 +936,10 @@ one entry of `apps` in `internal/coreruleset/coreruleset.go`. gitea's fixes are:
         - `q` of a Conan search, for the rule for SQL injection (942100), which
           refused the pattern `zlib/*`. `q` skips it only when the request has
           one `q`, in the query string or the body. That `q` has the shape of a
-          Conan reference. A reference is a name, then optionally `/` and a
+          Conan reference and holds no `*/`, which ends an SQL comment. A
-          version, then optionally `@` and a user, then optionally `/` and a
+          reference is a name, then optionally `/` and a version, then
-          channel. Each part is made of letters, digits, `_`, `.`, `-` and `*`.
+          optionally `@` and a user, then optionally `/` and a channel. Each
-          A part holds no `+`, since 942100 reads `+` as a space. `q` also holds
+          part is made of letters, digits, `_`, `.`, `+`, `-` and `*`.
-          no `*/`, which ends an SQL comment, and no `--`, which starts one. And
-          942100 finds no SQL injection in `q` once each comment that `/*`
-          starts, an unclosed one included, is replaced by a space.
 
     - The path of these requests, which names a file:
         - a file's page;

internal/coreruleset/gitea.conf

@@ -272,18 +272,9 @@ SecRule REQUEST_FILENAME "@rx ^/api/packages/[^/]+/npm/-/v1/search$" \
 # one q, in the query string or the body, with the shape of a Conan reference.
 # A reference is a name, then optionally / and a version, then optionally @
 # and a user, then optionally / and a channel. Each part is made of letters,
-# digits, _, ., - and *. A part holds no +, since 942100 reads + as a space.
+# digits, _, ., +, - and *. q must also hold no */, which ends an SQL comment.
-# q must also hold no */, which ends an SQL comment, and no --, which starts
+# Such a q has no quote, no space and no comment between two words, so SQL
-# one. Last, 942100 must find no SQL injection in q once each comment that /*
+# injection such as 1' OR '1'='1, 1/**/or/**/1 or 1/*@*/or-1 stays refused.
-# starts, an unclosed one included, is replaced by a space. So zlib/* passes,
-# and SQL injection such as these stays refused:
-#   1' OR '1'='1
-#   1/**/or/**/1
-#   1/*@*/or-1
-#   1+or+1--
-#   1-or-1--
-#   x/1.or.1
-#   1.having.1
 # This rule is in phase 2, because a phase 1 rule runs before the body is read
 # and cannot see a q there. It checks tx.app_gitea, since the skip above does
 # not reach phase 2.
@@ -292,13 +283,11 @@ SecRule REQUEST_FILENAME "@rx ^/api/packages/[^/]+/conan/v[12]/conans/search$" \
     SecRule &TX:app_gitea "@eq 1" "chain"
     SecRule &ARGS:q "@eq 1" "chain"
     SecRule ARGS:q "!@contains */" "chain"
-    SecRule ARGS:q "!@contains --" "chain"
+    SecRule ARGS:q "@rx ^[\w.+*-]+(?:/[\w.+*-]+)?(?:@[\w.+*-]+(?:/[\w.+*-]+)?)?$" \
-    SecRule ARGS:q "!@detectSQLi" "t:none,t:replaceComments,chain"
-    SecRule ARGS:q "@rx ^[\w.*-]+(?:/[\w.*-]+)?(?:@[\w.*-]+(?:/[\w.*-]+)?)?$" \
         "ctl:ruleRemoveTargetById=942100;ARGS:q"
 
-# The NuGet v2 API's searches, Search() and Packages(), send their words in
+# The NuGet registry's older searches, Search() and Packages(), send their words
-# searchTerm, or in $filter as an expression such as
+# in searchTerm, or in $filter as an expression such as
 # tolower(Id) eq 'python3'. Search()/$count and Packages()/$count count a
 # search's results and read the same fields. searchTerm skips these rules:
 #   930120: .htaccess

internal/coreruleset/gitea_test.go

@@ -384,12 +384,12 @@ var giteaRegistrySearches = []string{
 	"/api/packages/alice/nuget/Packages()/$count?$filter=",
 }
 
-// giteaRegistrySearchesLettingBinSh are the package registries' searches
+// giteaRegistryNameSearches are the package registries' searches whose field
-// whose field also lets /bin/sh through, since a Composer, Conan or npm name
+// also lets /bin/sh through, since a Composer, Conan or npm name can hold a
-// can hold a slash.
+// slash.
 //
 //nolint:gochecknoglobals // a constant cannot be a list
-var giteaRegistrySearchesLettingBinSh = []string{
+var giteaRegistryNameSearches = []string{
 	"/api/packages/alice/composer/search.json?q=",
 	"/api/packages/alice/conan/v1/conans/search?q=",
 	"/api/packages/alice/conan/v2/conans/search?q=",
@@ -402,7 +402,7 @@ func TestGiteaAttacksAreRefusedInItsRegistrySearches(t *testing.T) {
 	checkSearchesRefuseAttacks(
 		t,
 		giteaRegistrySearches,
-		giteaRegistrySearchesLettingBinSh,
+		giteaRegistryNameSearches,
 	)
 }
 
@@ -441,8 +441,7 @@ func TestGiteaRegistrySearchExclusionsAreNarrow(t *testing.T) {
 	// another operator on NuGet's searches, and $filter on NuGet's other
 	// paths. The rule for SQL injection (942100) still refuses tools/* in a
 	// field other than a Conan search's, SQL injection in a second q after a
-	// Conan pattern, SQL comments or dots that stand in for spaces, and SQL
+	// Conan pattern, and SQL comments that stand in for spaces.
-	// injection holding + or --.
 	for _, target := range []string{
 		"/api/packages/alice/nuget/Search()?$where=1",
 		"/api/packages/alice/nuget/Packages()?$ne=1",
@@ -455,11 +454,6 @@ func TestGiteaRegistrySearchExclusionsAreNarrow(t *testing.T) {
 			sqlInjection,
 		"/api/packages/alice/conan/v2/conans/search?q=1%2F%2A%2A%2For%2F%2A%2A%2F1",
 		"/api/packages/alice/conan/v2/conans/search?q=1%2F%2A%40%2A%2For-1",
-		"/api/packages/alice/conan/v2/conans/search?q=1%2Bor%2B1--",
-		"/api/packages/alice/conan/v2/conans/search?q=1-or-1--",
-		"/api/packages/alice/conan/v2/conans/search?q=x%2F1.or.1",
-		"/api/packages/alice/conan/v2/conans/search?q=x%2F1.and.1",
-		"/api/packages/alice/conan/v2/conans/search?q=1.having.1",
 	} {
 		if got := inspect(t, crs, get(target)); got.Score < threshold {
 			t.Errorf("%s let through: %+v", target, got)
@@ -479,14 +473,6 @@ func TestGiteaRegistrySearchExclusionsAreNarrow(t *testing.T) {
 		t.Errorf("POST %s %s let through: %+v", search.target, body, got)
 	}
 
-	// So is SQL injection with dots for spaces, in a q sent only in the body.
-	search.target = "/api/packages/alice/conan/v2/conans/search"
-
-	body = "q=x%2F1.or.1"
-	if got, _ := inspectBody(t, readingBodies(t), search, body); got.Score < threshold {
-		t.Errorf("POST %s %s let through: %+v", search.target, body, got)
-	}
-
 	// The rule that lets a Conan pattern through is in phase 2, where it
 	// checks tx.app_gitea itself: a hostname that does not serve gitea
 	// refuses the pattern.