fix: multipart body processor was closing the streams

actions/prepend.go

@@ -41,7 +41,10 @@ func (a *prependFn) Evaluate(r *coraza.Rule, tx *coraza.Transaction) {
 	if err != nil {
 		tx.Waf.Logger.Debug("failed to write buffer while evaluating prepend action")
 	}
-	reader := tx.ResponseBodyBuffer.Reader()
+	reader, err := tx.ResponseBodyBuffer.Reader()
+	if err != nil {
+		tx.Waf.Logger.Debug("failed to read response body while evaluating prepend action")
+	}
 	_, err = io.Copy(buf, reader)
 	if err != nil {
 		tx.Waf.Logger.Debug("failed to append response buffer while evaluating prepend action")

body_buffer.go

@@ -56,12 +56,14 @@ func (br *BodyBuffer) Write(data []byte) (n int, err error) {
 }
 
 // Reader Returns a working reader for the body buffer in memory or file
-func (br *BodyBuffer) Reader() io.Reader {
+func (br *BodyBuffer) Reader() (io.Reader, error) {
 	if br.writer == nil {
-		return bytes.NewReader(br.buffer.Bytes())
+		return bytes.NewReader(br.buffer.Bytes()), nil
 	}
-	_, _ = br.writer.Seek(0, 0)
-	return br.writer
+	if _, err := br.writer.Seek(0, 0); err != nil {
+		return nil, err
+	}
+	return br.writer, nil
 }
 
 // Size returns the current size of the body buffer

body_buffer_test.go

@@ -27,7 +27,11 @@ func TestBodyReaderMemory(t *testing.T) {
 		t.Error(err)
 	}
 	buf := new(strings.Builder)
-	if _, err := io.Copy(buf, br.Reader()); err != nil {
+	reader, err := br.Reader()
+	if err != nil {
+		t.Error(err)
+	}
+	if _, err := io.Copy(buf, reader); err != nil {
 		t.Error(err)
 	}
 	if buf.String() != "test" {
@@ -43,7 +47,11 @@ func TestBodyReaderFile(t *testing.T) {
 		t.Error(err)
 	}
 	buf := new(strings.Builder)
-	if _, err := io.Copy(buf, br.Reader()); err != nil {
+	reader, err := br.Reader()
+	if err != nil {
+		t.Error(err)
+	}
+	if _, err := io.Copy(buf, reader); err != nil {
 		t.Error(err)
 	}
 	if buf.String() != "test" {
@@ -67,7 +75,11 @@ func TestBodyReaderWriteFromReader(t *testing.T) {
 		t.Error(err)
 	}
 	buf := new(strings.Builder)
-	if _, err := io.Copy(buf, br.Reader()); err != nil {
+	reader, err := br.Reader()
+	if err != nil {
+		t.Error(err)
+	}
+	if _, err := io.Copy(buf, reader); err != nil {
 		t.Error(err)
 	}
 	if buf.String() != "test" {

bodyprocessors/multipart.go

@@ -30,7 +30,6 @@ func (mbp *multipartBodyProcessor) Read(reader io.Reader, mime string, storagePa
 	req, _ := http.NewRequest("GET", "/", reader)
 	req.Header.Set("Content-Type", mime)
 	err := req.ParseMultipartForm(1000000000)
-	defer req.Body.Close()
 	if err != nil {
 		return err
 	}

transaction.go

@@ -506,7 +506,10 @@ func (tx *Transaction) ProcessRequest(req *http.Request) (*types.Interruption, e
 		if err != nil {
 			return tx.Interruption, err
 		}
-		reader := tx.RequestBodyBuffer.Reader()
+		reader, err := tx.RequestBodyBuffer.Reader()
+		if err != nil {
+			return tx.Interruption, err
+		}
 		req.Body = io.NopCloser(reader)
 	}
 	return tx.ProcessRequestBody()
@@ -671,7 +674,10 @@ func (tx *Transaction) ProcessRequestBody() (*types.Interruption, error) {
 		mime = m[0]
 	}
 
-	reader := tx.RequestBodyBuffer.Reader()
+	reader, err := tx.RequestBodyBuffer.Reader()
+	if err != nil {
+		return nil, err
+	}
 
 	// Chunked requests will always be written to a temporary file
 	if tx.RequestBodyBuffer.Size() >= tx.RequestBodyLimit {
@@ -790,7 +796,10 @@ func (tx *Transaction) ProcessResponseBody() (*types.Interruption, error) {
 		tx.Waf.Rules.Eval(types.PhaseResponseBody, tx)
 		return tx.Interruption, nil
 	}
-	reader := tx.ResponseBodyBuffer.Reader()
+	reader, err := tx.ResponseBodyBuffer.Reader()
+	if err != nil {
+		return nil, err
+	}
 	reader = io.LimitReader(reader, tx.Waf.ResponseBodyLimit)
 	buf := new(strings.Builder)
 	length, _ := io.Copy(buf, reader)

transaction_test.go

@@ -15,8 +15,14 @@
 package coraza
 
 import (
+	"bufio"
+	"bytes"
 	"fmt"
+	"io"
+	"io/ioutil"
+	"mime/multipart"
 	"net/http"
+	"os"
 	"regexp"
 	"strconv"
 	"strings"
@@ -391,6 +397,55 @@ func TestAuditLogMessages(t *testing.T) {
 
 }
 
+func TestProcessRequestMultipart(t *testing.T) {
+	req, _ := http.NewRequest("POST", "/some", nil)
+	if err := multipartRequest(req); err != nil {
+		t.Fatal(err)
+	}
+	tx := makeTransaction()
+	tx.RequestBodyAccess = true
+	if _, err := tx.ProcessRequest(req); err != nil {
+		t.Error(err)
+	}
+	if req.Body == nil {
+		t.Error("failed to process multipart request")
+	}
+	reader := bufio.NewReader(req.Body)
+	if _, err := reader.ReadString('\n'); err != nil {
+		t.Error("failed to read multipart request", err)
+	}
+}
+
+func multipartRequest(req *http.Request) error {
+	var b bytes.Buffer
+	w := multipart.NewWriter(&b)
+	tempfile, err := os.CreateTemp("/tmp", "tmpfile*")
+	if err != nil {
+		return err
+	}
+	defer os.Remove(tempfile.Name())
+	for i := 0; i < 1024*5; i++ {
+		// this should create a 5mb file
+		if _, err := tempfile.Write([]byte(strings.Repeat("A", 1024))); err != nil {
+			return err
+		}
+	}
+	var fw io.Writer
+	if fw, err = w.CreateFormFile("fupload", tempfile.Name()); err != nil {
+		return err
+	}
+	if _, err := tempfile.Seek(0, 0); err != nil {
+		return err
+	}
+	if _, err = io.Copy(fw, tempfile); err != nil {
+		return err
+	}
+	req.Body = ioutil.NopCloser(&b)
+	req.Header.Set("Content-Type", w.FormDataContentType())
+	req.Method = "POST"
+	return nil
+}
+
 func BenchmarkTransactionCreation(b *testing.B) {
 	waf := NewWaf()
 	for i := 0; i < b.N; i++ {