Guard against negative dataLength while reading a page (#763)

* Added guard code

Signed-off-by: Joe Elliott <number101010@gmail.com>

* changelog

Signed-off-by: Joe Elliott <number101010@gmail.com>

* test

Signed-off-by: Joe Elliott <number101010@gmail.com>

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## main / unreleased
 
 * [FEATURE] Added the ability to hedge requests with all backends [#750](https://github.com/grafana/tempo/pull/750) (@joe-elliott)
+* [BUGFIX] Guard against negative dataLength [#763](https://github.com/grafana/tempo/pull/763) (@joe-elliott)
 
 ## v1.0.0
 

integration/microservices/docker-compose.yaml

@@ -16,6 +16,7 @@ services:
   ingester-0:
     image: tempo:latest
     command: "-target=ingester -config.file=/etc/tempo.yaml"
+    restart: always
     volumes:
       - ./tempo.yaml:/etc/tempo.yaml
     ports:
@@ -28,6 +29,7 @@ services:
   ingester-1:
     image: tempo:latest
     command: "-target=ingester -config.file=/etc/tempo.yaml"
+    restart: always
     volumes:
       - ./tempo.yaml:/etc/tempo.yaml
     ports:
@@ -40,6 +42,7 @@ services:
   ingester-2:
     image: tempo:latest
     command: "-target=ingester -config.file=/etc/tempo.yaml"
+    restart: always
     volumes:
       - ./tempo.yaml:/etc/tempo.yaml
     ports:
@@ -50,7 +53,7 @@ services:
       - distributor # inverted relationship here to add delay for minio
 
   minio:
-    image: minio/minio:RELEASE.2021-05-26T00-22-46Z
+    image: minio/minio:RELEASE.2021-06-14T01-29-23Z
     environment:
       - MINIO_ACCESS_KEY=tempo
       - MINIO_SECRET_KEY=supersecret

tempodb/encoding/v2/page.go

@@ -78,6 +78,10 @@ func unmarshalPageFromReader(r io.Reader, header pageHeader, buffer []byte) (*pa
 	}
 	dataLength := int(totalLength) - totalHeaderSize
 
+	if dataLength < 0 {
+		return nil, fmt.Errorf("unexpected negative dataLength unmarshalling page: %d", dataLength)
+	}
+
 	if cap(buffer) < dataLength {
 		buffer = make([]byte, dataLength)
 	} else {

tempodb/encoding/v2/page_test.go

@@ -16,7 +16,7 @@ type testHeader struct {
 
 func (h *testHeader) unmarshalHeader(b []byte) error {
 	if len(b) != int(uint32Size) {
-		return errors.New("err")
+		return errors.New("testheader err")
 	}
 
 	h.field = binary.LittleEndian.Uint32(b[:uint32Size])
@@ -108,3 +108,26 @@ func TestPageMarshalToBuffer(t *testing.T) {
 		assert.Equal(t, tc.field, page.header.(*testHeader).field)
 	}
 }
+
+func TestCorruptHeader(t *testing.T) {
+	data := []byte{0x01, 0x02, 0x03}
+	buff := &bytes.Buffer{}
+
+	bytesWritten, err := marshalPageToWriter(data, buff, constDataHeader)
+	require.NoError(t, err)
+	assert.Equal(t, len(data)+constDataHeader.headerLength()+int(baseHeaderSize), bytesWritten)
+
+	buffBytes := buff.Bytes()
+
+	// overwrite the base header with 0s
+	zeroHeader := bytes.Repeat([]byte{0x00}, baseHeaderSize)
+	copy(buffBytes, zeroHeader)
+
+	page, err := unmarshalPageFromBytes(buffBytes, constDataHeader)
+	assert.Nil(t, page)
+	assert.EqualError(t, err, "expected data len -6 does not match actual 3")
+
+	page, err = unmarshalPageFromReader(bytes.NewReader(buffBytes), constDataHeader, nil)
+	assert.Nil(t, page)
+	assert.EqualError(t, err, "unexpected negative dataLength unmarshalling page: -6")
+}