diff --git a/internal/bodyprocessors/argumentslimit_test.go b/internal/bodyprocessors/argumentslimit_test.go index 872d5bbc6..b4ceb9396 100644 --- a/internal/bodyprocessors/argumentslimit_test.go +++ b/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("") + xmlrpc := strings.Builder{} + xmlrpc.WriteString(`system.multicall`) + for i := 0; i < 300; i++ { + fmt.Fprintf(&xmlrpc, `value %d`, i) + } + xmlrpc.WriteString(``) + 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{}) diff --git a/internal/bodyprocessors/xml.go b/internal/bodyprocessors/xml.go index bd7ab08a7..9f3120f8d 100644 --- a/internal/bodyprocessors/xml.go +++ b/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() diff --git a/internal/bodyprocessors/xml_test.go b/internal/bodyprocessors/xml_test.go index d71bda6e0..195f857ab 100644 --- a/internal/bodyprocessors/xml_test.go +++ b/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: ` + + wp.getUsersBlogs + + admin + hunter2 + + `, + Want: []string{"wp.getUsersBlogs", "admin", "hunter2"}, + }, + { + Name: "xmlRPCMulticall", + Input: ` + + system.multicall + + + methodNamewp.getCategories + + + `, + Want: []string{"system.multicall", "methodName", "wp.getCategories"}, + }, + { + Name: "atomLink", + Input: `alternate`, + Want: []string{"alternate"}, + }, + { + // Elements left unterminated still decode, so leniency for + // unbalanced documents does not depend on self-closing them. + Name: "unterminatedParam", + Input: `admin`, + Want: []string{"admin"}, + }, + { + Name: "unterminatedHTMLVoidElements", + Input: `before
after`, + 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