fix(http-honeypot): a semicolon hides the payload from four classifiers (#3364)

[Xore]

Closes #3364.

Two parts: the missing research document, and a root-cause parser fix the research exposed.

The bug

Since Go 1.17, url.ParseQuery rejects every pair containing a ; and drops it — but returns a non-nil map of the pairs that had none. Four call sites guarded with:

values, err := url.ParseQuery(raw)
if err != nil && len(values) == 0 { continue }

The guard does not fire (there are values left), so parsing continues on the truncated map with the attacker's pair simply absent. The thing the guard was written to tolerate — a body that is not a parameter list at all — is exactly what makes the miss invisible: there is no error to notice.

This matters because the targets are PHP, whose only separator is &. A ; in a value is ordinary data that reaches the target intact while the sensor drops the parameter carrying it. Confirmed end-to-end in the red test: the request body reaches the event (_user=admin%5C%27;or+1%3D1--) but arrives unclassified.

The fix

One root cause, four sites — all four swapped to the existing formValues helper (classify_wordpress.go, added by #3449 for exactly this reason). No new parser, no new file, no new abstraction:

• classify_roundcube.go, classify_odata.go, classify_teamcity.go, laundering.go

Each site carries a comment explaining why formValues and not url.ParseQuery, not just that the name changed.

Proof

I re-ran the red check independently: restored all four call sites to main's url.ParseQuery version, ran the new test, and got 7 failures, then restored. It is green on the branch.

The author also mutation-checked it: dropping formUnescape's undecodable-side case → 2 tests fail; re-introducing ; as a separator → 5 fail, including a pre-existing #3447 one; reverting each productive call site individually → each caught.

gofmt -l clean, go vet clean, full package ok with -count=1 (uncached). classify_order_3464_test.go passes unmodified and is not in the diff.

The one deliberate behaviour change — review this first

Three existing assertions moved, all on one fixture, $filter=Year%zz%2520eq, and not because of the semicolon.

url.ParseQuery returns an empty map on that broken escape, so the guard fired and the pair was skipped. formValues keeps the undecodable side, so the genuine %25 — a % that decodes twice — is now visible, and the target reads Year%zz%20eq. The old empty classification was the parser deleting its own evidence, not the gate declining.

The rule is unchanged: a malformed escape alone is still not a residual escape. That is now pinned separately on $filter=Year%zz%2Gz, a value whose only escape is broken. What changed is which requests reach the rule. The old test was asserting the parser's behaviour rather than the sensor's rule; that distinction is now two rows instead of one blurred row, justified at the pin in both affected test files.

This is a real change in what gets labelled. It is a widening — more requests qualify — and the reasoning is sound, but it is the thing to check if you disagree with the call.

Docs

docs/research/3364-roundcube-sqli.md carries the unmeasured-coverage caveat forward rather than papering over it: 0 claims against #1888's pinned fixture, 9/9 published shapes, explicitly labelled a fixture and not the fleet. No CVSS, build, or date asserted beyond the issue body and the class's own comment; AV26-503 is cited as second-hand because it was not re-fetched.

Author and committer both Xore <Xore@users.noreply.github.com>, no AI attribution trailers.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None