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

arcane/home/honeypot-http/http-honeypot/classify_odata.go

@@ -6,10 +6,7 @@ package main
 // state exists, which is why it is a case in the dispatch and not a hook
 // behind it.
 
-import (
-	"net/url"
-	"strings"
-)
+import "strings"
 
 // odataDoubleEncode reports an OData system query option -- $select, $filter,
 // $top and the rest -- whose key or value still carries a percent-escape
@@ -47,10 +44,14 @@ func odataDoubleEncode(query, body string) bool {
 		"$format": true, "$search": true, "$skiptoken": true, "$index": true,
 	}
 	for _, raw := range []string{query, body} {
-		values, err := url.ParseQuery(raw)
-		if err != nil && len(values) == 0 {
-			continue
-		}
+		// formValues, not url.ParseQuery, for the reason
+		// roundcubeVirtuserSQLi gives at its own call site: a `;` inside
+		// a pair makes url.ParseQuery drop that pair, so an option whose
+		// own value carries one was invisible to this gate while the
+		// target parsed it. `;` is also what a Java/ASP.NET-style
+		// client sends when it is being careless, and a request nobody
+		// sends is not evidence.
+		values := formValues(raw)
 		for key, vals := range values {
 			// OData allows a namespace alias prefix, so compare the last
 			// dotted part: `northwind.$filter` is the same option.
@@ -75,8 +76,8 @@ func odataDoubleEncode(query, body string) bool {
 }
 
 // odataDoubleEncodeCase is the dispatch's entry for odataDoubleEncode. The
-// raw query and body go in, not the lowercased pair: url.ParseQuery does its
-// own decoding, and the residue this class is looking for is destroyed by a
+// raw query and body go in, not the lowercased pair: formValues does its own
+// decoding, and the residue this class is looking for is destroyed by a
 // decode that happens before it.
 func odataDoubleEncodeCase(c classifyInput) bool {
 	return odataDoubleEncode(c.Query, c.Body)

arcane/home/honeypot-http/http-honeypot/classify_roundcube.go

@@ -4,10 +4,7 @@ package main
 // change to it touches one new-ish file and one line of the dispatch in
 // classify.go, and nothing else in this package.
 
-import (
-	"net/url"
-	"strings"
-)
+import "strings"
 
 // roundcubeVirtuserSQLi reports a pre-authentication SQL injection aimed at
 // Roundcube Webmail's virtuser_query plugin (CVE-2026-48842). CVSS 8.1; the
@@ -43,8 +40,8 @@ import (
 // wordpressPagenameTraversal gives: the text "virtuser_query" inside some
 // other value must not trigger this, and neither must a payload that happens
 // to contain "_task". Nothing is deserialized and nothing is evaluated --
-// url.ParseQuery splits a string into key/value pairs, and every value is
-// then matched as bytes.
+// formValues splits a string into key/value pairs the way PHP will split it,
+// and every value is then matched as bytes.
 //
 // Its place in the dispatch is first, ahead of the generic sqli class,
 // because a Roundcube probe is often both at once -- the same value is caught
@@ -65,10 +62,13 @@ import (
 // a fixed number of linear passes over bytes already in memory.
 func roundcubeVirtuserSQLi(c classifyInput) bool {
 	for _, raw := range []string{c.Query, c.Body} {
-		values, err := url.ParseQuery(raw)
-		if err != nil && len(values) == 0 {
-			continue
-		}
+		// formValues, not url.ParseQuery: the target is PHP, whose only
+		// separator is "&", so a `;` inside a value is data that reaches
+		// the plugin intact -- while url.ParseQuery rejects the pair
+		// carrying it and hands back a map with the parameter missing.
+		// The payload would arrive at Roundcube and never reach this
+		// case. See formValues, and form_values_semicolon_3364_test.go.
+		values := formValues(raw)
 		roundcube := false
 		for key, vals := range values {
 			// The dispatch parameters, and the plugin named as a key.

arcane/home/honeypot-http/http-honeypot/classify_teamcity.go

@@ -5,10 +5,7 @@ package main
 // ordering decision #3444 had to make by hand and the one this file's own
 // tests now pin from both sides.
 
-import (
-	"net/url"
-	"strings"
-)
+import "strings"
 
 // teamcityAgentDeserialization reports a request aimed at a Java
 // deserialization sink through TeamCity's build-agent polling protocol
@@ -236,7 +233,7 @@ var teamcityProtocolMarkers = []string{
 // xmlrpc/allowRegistrationAndPing, and a call name sitting inside somebody
 // else's parameter value, both keep their own answers.
 //
-// Nothing is parsed in order to answer this. url.ParseQuery splits a string
+// Nothing is parsed in order to answer this. formValues splits a string
 // into key/value pairs and the element extractor is two index searches;
 // neither is told what to do with what it found, and neither can fail in a
 // way that changes the answer. Parsing a request to learn what it asked for
@@ -250,12 +247,22 @@ func teamcityAgentProtocol(lowerQuery, lowerBody string) bool {
 	for _, channel := range []string{lowerQuery, lowerBody} {
 		// A body that is not a parameter list at all parses into junk keys
 		// and values, which is harmless: the key test below rejects them.
-		// An error alongside real values is tolerated for the same reason
-		// wordpressPagenameTraversal tolerates it.
-		values, err := url.ParseQuery(channel)
-		if err != nil && len(values) == 0 {
-			continue
-		}
+		// formValues, for the reason the other three call sites give: a
+		// `;` inside a pair must not delete the pair.
+		//
+		// This site is the exception on the evidence, and the exception is
+		// in the call name, not the parser. teamcityAgentCall compares a
+		// value to a fixed list as a whole string, so a `;` in the value
+		// makes it a different value rather than the same one the parser
+		// dropped, and a `;` in front of the key makes it a different key
+		// rather than the parameter it resembles. There is no semicolon
+		// payload this class can be shown to gain here: the target's parser
+		// reaches the same verdict. The swap is for consistency with the
+		// other three sites and to retire one root cause rather than four,
+		// and it is pinned as gaining nothing in
+		// form_values_semicolon_3364_test.go so that "nothing to gain" is
+		// a measured claim rather than an assumption.
+		values := formValues(channel)
 		for key, vals := range values {
 			if key != "methodname" && key != "method" {
 				continue