fix: blocks body buffer reader once the body buffer has been reset.

Currently, the body buffer would hold a piece of the body, hence a reader will be sent to the connector in order to pass it to upstream. The problem happens when the transaction is closed, the body buffer is resetted but the reader is still out there and the connector tries to drain it (using something like io.Copy(io.Discard, body) in order to reuse the connection, the body buffer is already empty (lenght 0) but the body buffer reader points to the end of the buffer (e.g. 512), hence attempting to read it till the end (from 512 to 0) trigger an out of range error.

Closes corazawaf/coraza-caddy#48.

go.work.sum

@@ -75,6 +75,7 @@ github.com/hashicorp/serf v0.9.6 h1:uuEX1kLR6aoda1TBttmJQKDLZE1Ob7KN0NPdE7EtCDc=
 github.com/hashicorp/vault/api v1.0.4 h1:j08Or/wryXT4AcHj1oCbMd7IijXcKzYUGw59LGu9onU=
 github.com/hashicorp/vault/sdk v0.1.13 h1:mOEPeOhT7jl0J4AMl1E705+BcmeRs1VmKNb9F0sMLy8=
 github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d h1:kJCB4vdITiW1eC1vq2e6IsrXKrZit1bv/TDYFGMp4BQ=
+github.com/inconshreveable/mousetrap v1.0.1/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
 github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
 github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8=
 github.com/jpillora/backoff v1.0.0 h1:uvFg412JmmHBHw7iwprIxkPMI+sGQ4kzOWsMeHnm2EA=
@@ -125,7 +126,6 @@ go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4=
 go.uber.org/zap v1.17.0 h1:MTjgFu6ZLKvY6Pvaqk97GlxNBuMpV4Hy/3P6tRGlI2U=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 h1:7I4JAnoQBe7ZtJcBaYHi5UtiO8tQHbUSXxL+pnGRANg=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
-golang.org/x/crypto v0.10.0 h1:LKqV2xt9+kDzSTfOhx4FrkEBcMrAgHSYgzywV9zcGmM=
 golang.org/x/crypto v0.10.0/go.mod h1:o4eNf7Ede1fv+hwOwZsTHl9EsPFO6q6ZvYR8vYfY45I=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4 h1:c2HOrn5iMezYjSlGPncknSEr/8x5LELb/ilJbXi9DEA=
 golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug=
@@ -158,6 +158,7 @@ golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo=
 golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
 golang.org/x/text v0.8.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
 golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
+golang.org/x/text v0.10.0 h1:UpjohKhiEgNc0CSauXmwYftY1+LlaC75SJwh0SgCX58=
 golang.org/x/text v0.10.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 h1:SvFZT6jyqRaOeXpc5h/JSfZenJ2O330aBsf7JfSUXmQ=
 google.golang.org/appengine v1.4.0 h1:/wp5JvzpHIxhs/dumFmF7BXTf3Z+dd4uXta4kVyO508=

internal/corazawaf/body_buffer.go

@@ -21,6 +21,7 @@ type BodyBuffer struct {
 	buffer  *bytes.Buffer
 	writer  *os.File
 	length  int64
+	readers []*bodyBufferReader
 }
 
 var (
@@ -57,7 +58,7 @@ func (br *BodyBuffer) Write(data []byte) (n int, err error) {
 		// Write has been called without checking the Limit, it should never happend. Raising an error.
 		// The buffers are private and populated only by WriteRequestBody, ReadRequestBodyFrom, and similar functions
 		// that have to perform limit checks before calling Write()
-		return 0, errors.New("Limit reached while writing")
+		return 0, errors.New("limit reached while writing")
 	}
 	targetLen := br.length + int64(len(data))
 
@@ -99,30 +100,45 @@ type bodyBufferReader struct {
 }
 
 func (b *bodyBufferReader) Read(p []byte) (n int, err error) {
+	if b.br == nil {
+		// reader has been closed and hence we don't attempt to do anymore read
+		return 0, io.EOF
+	}
+
 	if !environment.HasAccessToFS || b.br.writer == nil {
 		buf := b.br.buffer.Bytes()
+
 		n = len(p)
-		if b.pos+n > len(buf) {
-			n = len(buf) - b.pos
+		if bl := b.br.buffer.Len(); b.pos+n > bl {
+			n = bl - b.pos
 		}
 		if n == 0 {
 			return 0, io.EOF
 		}
-		copy(p, buf[b.pos:b.pos+n])
-		b.pos += n
-		return
+
+		k := copy(p, buf[b.pos:b.pos+n])
+		b.pos += k
+		return k, nil
 	}
 
 	n, err = b.br.writer.ReadAt(p, int64(b.pos))
 	b.pos += n
 	return
 }
 
+// Close closes the reader
+func (b *bodyBufferReader) Close() {
+	b.br = nil
+	b.pos = 0
+}
+
 // Reader Returns a working reader for the body buffer in memory or file
 func (br *BodyBuffer) Reader() (io.Reader, error) {
-	return &bodyBufferReader{
+	r := &bodyBufferReader{
 		br: br,
-	}, nil
+	}
+	br.readers = append(br.readers, r)
+	return r, nil
 }
 
 // Size returns the current size of the body buffer
@@ -134,6 +150,15 @@ func (br *BodyBuffer) Size() int64 {
 func (br *BodyBuffer) Reset() error {
 	br.buffer.Reset()
 	br.length = 0
+
+	// close all readers, this is important because connectors may have
+	// a reference to a reader but the transaction is already closed and
+	// hence the reader is not valid anymore.
+	for _, r := range br.readers {
+		r.Close()
+	}
+	br.readers = nil
+
 	if environment.HasAccessToFS && br.writer != nil {
 		w := br.writer
 		br.writer = nil

internal/corazawaf/body_buffer_test.go

@@ -141,3 +141,36 @@ func TestWriteLimit(t *testing.T) {
 		})
 	}
 }
+
+// See https://github.com/corazawaf/coraza-caddy/issues/48
+func TestBodyBufferResetAndReadTheReader(t *testing.T) {
+	br := NewBodyBuffer(types.BodyBufferOptions{
+		MemoryLimit: 5,
+		Limit:       5,
+	})
+	br.Write([]byte("test1"))
+
+	r, _ := br.Reader()
+
+	dest := make([]byte, 5)
+	nRead, err := r.Read(dest)
+	if err != nil {
+		t.Fatalf("unexpected error when creating reader %s", err.Error())
+	}
+	if nRead != 5 {
+		t.Fatalf("unexpected number of bytes read, want: %d, have: %d", 5, nRead)
+	}
+
+	err = br.Reset()
+	if err != nil {
+		t.Fatalf("unexpected error %s", err.Error())
+	}
+
+	nCopied, err := io.Copy(io.Discard, r)
+	if err != nil {
+		t.Fatalf("unexpected error %s", err.Error())
+	}
+	if nCopied != 0 {
+		t.Fatalf("unexpected number of bytes read, want: %d, have: %d", 5, nCopied)
+	}
+}