fix: parse XML bodies whose elements are named after HTML void elements

internal/bodyprocessors/argumentslimit_test.go

@@ -212,8 +212,10 @@ func TestXMLArgumentsLimitSharedBudget(t *testing.T) {
 
 // TestXMLOrdinaryDocumentsAccepted asserts that documents an ordinary client
 // sends parse whole at the shipped default limit: a SOAP response of several
-// hundred records and a configuration document of several hundred elements
-// both hold more nodes than there are arguments in the budget.
+// hundred records, a configuration document of several hundred elements and an
+// XML-RPC multicall of several hundred parameters all hold more nodes than
+// there are arguments in the budget. Reaching ProcessRequest without an error
+// is what keeps REQUEST_XML populated and REQBODY_ERROR clear.
 func TestXMLOrdinaryDocumentsAccepted(t *testing.T) {
 	bp, err := bodyprocessors.GetBodyProcessor("xml")
 	if err != nil {
@@ -233,12 +235,20 @@ func TestXMLOrdinaryDocumentsAccepted(t *testing.T) {
 	}
 	config.WriteString("</configuration>")
 
+	xmlrpc := strings.Builder{}
+	xmlrpc.WriteString(`<?xml version="1.0"?><methodCall><methodName>system.multicall</methodName><params>`)
+	for i := 0; i < 300; i++ {
+		fmt.Fprintf(&xmlrpc, `<param><value><string>value %d</string></value></param>`, i)
+	}
+	xmlrpc.WriteString(`</params></methodCall>`)
+
 	for _, tc := range []struct {
 		name string
 		body string
 	}{
 		{name: "soap_records", body: soap.String()},
 		{name: "config_elements", body: config.String()},
+		{name: "xmlrpc_multicall", body: xmlrpc.String()},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			v := corazawaf.NewTransactionVariables(persistence.NoopEngine{})

internal/bodyprocessors/xml.go

@@ -50,8 +50,14 @@ func readXML(reader io.Reader, limit int) ([]string, []string, error) {
 	var attrs []string
 	var content []string
 	dec := xml.NewDecoder(reader)
+	// Strict false is what lets an element be closed by an ancestor's end tag
+	// rather than its own, so an unterminated element costs only itself; a body
+	// cut short is tolerated separately, by the unexpected EOF branch below.
+	// AutoClose stays unset: it self-closes the elements on Go's HTML void list,
+	// and param, link, input and col are ordinary container elements in XML-RPC,
+	// Atom and SOAP bodies, whose real end tag would then abort decoding as
+	// unexpected.
 	dec.Strict = false
-	dec.AutoClose = xml.HTMLAutoClose
 	dec.Entity = xml.HTMLEntity
 	for {
 		token, err := dec.Token()

internal/bodyprocessors/xml_test.go

@@ -69,6 +69,75 @@ func TestXMLPayloadFlexibility(t *testing.T) {
 	}
 }
 
+// Elements named after HTML void elements are ordinary containers in XML
+// vocabularies, and their end tag must not abort decoding.
+func TestXMLHTMLVoidElementNames(t *testing.T) {
+	testCases := []struct {
+		Name  string
+		Input string
+		Want  []string
+	}{
+		{
+			Name: "xmlRPCGetUsersBlogs",
+			Input: `<?xml version="1.0"?>
+			<methodCall>
+			  <methodName>wp.getUsersBlogs</methodName>
+			  <params>
+				<param><value><string>admin</string></value></param>
+				<param><value><string>hunter2</string></value></param>
+			  </params>
+			</methodCall>`,
+			Want: []string{"wp.getUsersBlogs", "admin", "hunter2"},
+		},
+		{
+			Name: "xmlRPCMulticall",
+			Input: `<?xml version="1.0"?>
+			<methodCall>
+			  <methodName>system.multicall</methodName>
+			  <params><param><value><array><data>
+				<value><struct>
+				  <member><name>methodName</name><value><string>wp.getCategories</string></value></member>
+				</struct></value>
+			  </data></array></value></param></params>
+			</methodCall>`,
+			Want: []string{"system.multicall", "methodName", "wp.getCategories"},
+		},
+		{
+			Name:  "atomLink",
+			Input: `<entry><link href="https://example.com">alternate</link></entry>`,
+			Want:  []string{"alternate"},
+		},
+		{
+			// Elements left unterminated still decode, so leniency for
+			// unbalanced documents does not depend on self-closing them.
+			Name:  "unterminatedParam",
+			Input: `<methodCall><params><param><value>admin</value></params></methodCall>`,
+			Want:  []string{"admin"},
+		},
+		{
+			Name:  "unterminatedHTMLVoidElements",
+			Input: `<html><body>before<br>after<img src="x"></body></html>`,
+			Want:  []string{"before", "after"},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.Name, func(t *testing.T) {
+			_, contents, err := readXML(bytes.NewReader([]byte(tc.Input)), 0)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if got, want := len(contents), len(tc.Want); got != want {
+				t.Fatalf("contents count mismatch, got=%d (%v), want=%d", got, contents, want)
+			}
+			for i := range contents {
+				if got, want := contents[i], tc.Want[i]; got != want {
+					t.Errorf("Expected content got=%s, want=%s", got, want)
+				}
+			}
+		})
+	}
+}
+
 func TestXMLUnexpectedEOF(t *testing.T) {
 	testCases := []struct {
 		Name  string