Merge pull request #418 from AikidoSec/parse-json-and-forms

Author: bitterpanda63

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)

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{}

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
 }

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"}`