From 8e235b48bfce507bb4f724d36f6606da60b7eb5c Mon Sep 17 00:00:00 2001 From: Tom Aisthorpe Date: Wed, 22 Apr 2026 12:06:54 +0100 Subject: [PATCH] Attempt to parse both JSON and form bodies --- instrumentation/http/body.go | 19 +++---- instrumentation/http/body_test.go | 82 +++++++++++++++++++++++++++++++ instrumentation/http/json.go | 28 ++++++++--- instrumentation/http/json_test.go | 41 ++++++++++++++++ 4 files changed, 155 insertions(+), 15 deletions(-) diff --git a/instrumentation/http/body.go b/instrumentation/http/body.go index 81fc346a..e59a688f 100644 --- a/instrumentation/http/body.go +++ b/instrumentation/http/body.go @@ -17,24 +17,25 @@ type MultipartFormParser interface { MultipartForm() (*multipart.Form, error) } -// TryExtractBody attempts to extract body data from a request, trying JSON first, then forms +// TryExtractBody attempts to extract body data from a request using both JSON +// and form parsers, returning whichever finds data. Both are always attempted +// so the firewall does not depend on Content-Type to decide what the backend +// will process. func TryExtractBody(req *http.Request, parser MultipartFormParser) any { if req.Body == nil || req.Body == http.NoBody { return nil } bodyFromJSON := tryExtractJSON(req) + bodyFromForm := tryExtractFormBody(req, parser) + + if bodyFromJSON != nil && bodyFromForm != nil { + return []any{bodyFromJSON, bodyFromForm} + } if bodyFromJSON != nil { return bodyFromJSON } - - bodyFromForm := tryExtractFormBody(req, parser) - if bodyFromForm != nil { - return bodyFromForm - } - - // No usable data found, returning nil - return nil + return bodyFromForm } // tryExtractFormBody attempts to extract form data (urlencoded or multipart) diff --git a/instrumentation/http/body_test.go b/instrumentation/http/body_test.go index dc1c1134..f9a88e21 100644 --- a/instrumentation/http/body_test.go +++ b/instrumentation/http/body_test.go @@ -139,6 +139,88 @@ func TestTryExtractBody(t *testing.T) { }) } +func TestTryExtractBodyBypassVectors(t *testing.T) { + t.Run("empty JSON object prefix does not suppress multipart field extraction", func(t *testing.T) { + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + _ = writer.WriteField("name", "injected") + writer.Close() + + req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader("{}\n"+body.String())) + req.Header.Set("Content-Type", writer.FormDataContentType()) + + parser := &mockParser{req: req} + result := TryExtractBody(req, parser) + + formValues, ok := result.(url.Values) + require.True(t, ok, "expected url.Values for multipart/form-data request, got %T: %v", result, result) + assert.Equal(t, "injected", formValues.Get("name")) + }) + + t.Run("non-empty JSON object prefix does not suppress multipart field extraction", func(t *testing.T) { + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + _ = writer.WriteField("name", "injected") + writer.Close() + + req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(`{"key":"val"}`+"\n"+body.String())) + req.Header.Set("Content-Type", writer.FormDataContentType()) + + parser := &mockParser{req: req} + result := TryExtractBody(req, parser) + + formValues, ok := result.(url.Values) + require.True(t, ok, "expected url.Values for multipart/form-data request, got %T: %v", result, result) + assert.Equal(t, "injected", formValues.Get("name")) + }) + + t.Run("empty JSON array prefix does not suppress multipart field extraction", func(t *testing.T) { + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + _ = writer.WriteField("name", "injected") + writer.Close() + + req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader("[]\n"+body.String())) + req.Header.Set("Content-Type", writer.FormDataContentType()) + + parser := &mockParser{req: req} + result := TryExtractBody(req, parser) + + formValues, ok := result.(url.Values) + require.True(t, ok, "expected url.Values for multipart/form-data request, got %T: %v", result, result) + assert.Equal(t, "injected", formValues.Get("name")) + }) + + t.Run("JSON body is scanned even when Content-Type is multipart", func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(`{"name":"injected"}`)) + req.Header.Set("Content-Type", "multipart/form-data; boundary=----boundary") + + parser := &mockParser{req: req} + result := TryExtractBody(req, parser) + + resultMap, ok := result.(map[string]interface{}) + require.True(t, ok, "expected map, got %T: %v", result, result) + assert.Equal(t, "injected", resultMap["name"]) + }) + + t.Run("NDJSON body returns all objects for inspection", func(t *testing.T) { + body := `{"payload":"safe"}` + "\n" + `{"payload":"danger"}` + req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + + parser := &mockParser{req: req} + result := TryExtractBody(req, parser) + + resultSlice, ok := result.([]interface{}) + require.True(t, ok, "expected []interface{} for NDJSON body, got %T: %v", result, result) + require.Len(t, resultSlice, 2) + + second, ok := resultSlice[1].(map[string]interface{}) + require.True(t, ok, "expected second element to be a map") + assert.Equal(t, "danger", second["payload"]) + }) +} + func TestBodyStillReadableAfterExtraction(t *testing.T) { t.Run("body readable after form extraction", func(t *testing.T) { formData := url.Values{} diff --git a/instrumentation/http/json.go b/instrumentation/http/json.go index 114147b7..34618c12 100644 --- a/instrumentation/http/json.go +++ b/instrumentation/http/json.go @@ -3,6 +3,7 @@ package http import ( "bytes" "encoding/json" + "errors" "io" "net/http" ) @@ -11,18 +12,33 @@ func tryExtractJSON(r *http.Request) any { var buf bytes.Buffer tee := io.TeeReader(r.Body, &buf) - var data any - err := json.NewDecoder(tee).Decode(&data) + decoder := json.NewDecoder(tee) + var results []any + for { + var data any + err := decoder.Decode(&data) + if errors.Is(err, io.EOF) { + break + } + if err != nil { + _, _ = io.Copy(io.Discard, tee) + r.Body = io.NopCloser(&buf) + return nil + } + results = append(results, data) + } // Drain any remaining bytes to ensure full body is available in request // Ignore error - we still need to restore the request body _, _ = io.Copy(io.Discard, tee) - r.Body = io.NopCloser(&buf) - if err != nil { + switch len(results) { + case 0: return nil + case 1: + return results[0] + default: + return results } - - return data } diff --git a/instrumentation/http/json_test.go b/instrumentation/http/json_test.go index 9c46dc11..0c06e0be 100644 --- a/instrumentation/http/json_test.go +++ b/instrumentation/http/json_test.go @@ -5,8 +5,49 @@ import ( "net/http/httptest" "strings" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestTryExtractJSONStreamingBehavior(t *testing.T) { + t.Run("returns all objects when body contains multiple JSON objects", func(t *testing.T) { + body := `{"first":true}` + "\n" + `{"second":true}` + r := httptest.NewRequest("POST", "/test", strings.NewReader(body)) + + got := tryExtractJSON(r) + + gotSlice, ok := got.([]interface{}) + require.True(t, ok, "expected []interface{}, got %T: %v", got, got) + require.Len(t, gotSlice, 2) + assert.Equal(t, true, gotSlice[0].(map[string]interface{})["first"]) + assert.Equal(t, true, gotSlice[1].(map[string]interface{})["second"]) + + restoredBody, _ := io.ReadAll(r.Body) + assert.Equal(t, body, string(restoredBody)) + }) + + t.Run("returns nil when valid JSON is followed by non-JSON content", func(t *testing.T) { + multipartTrailer := "\n------boundary\r\nContent-Disposition: form-data; name=\"field\"\r\n\r\nvalue\r\n------boundary--" + body := "{}" + multipartTrailer + r := httptest.NewRequest("POST", "/test", strings.NewReader(body)) + + got := tryExtractJSON(r) + + assert.Nil(t, got) + }) + + t.Run("returns nil when valid JSON array is followed by non-JSON content", func(t *testing.T) { + multipartTrailer := "\n------boundary\r\nContent-Disposition: form-data; name=\"field\"\r\n\r\nvalue\r\n------boundary--" + body := "[]" + multipartTrailer + r := httptest.NewRequest("POST", "/test", strings.NewReader(body)) + + got := tryExtractJSON(r) + + assert.Nil(t, got) + }) +} + func TestTryExtractJSON(t *testing.T) { t.Run("good", func(t *testing.T) { body := `{"key": "value"}`