Skip to content

confighttp zstd decoder pooling causing data corruption #13954

@carsonip

Description

@carsonip

Component(s)

No response

What happened?

Describe the bug

There is a bug in config/confighttp zstd decoder pooling causing multiple copies of the same zstd decoder to exist in the sync.Pool. This happens when the reader Close is called multiple times which in turn returns the inner zstd decoder to be returned to the pool multiple times. (code)

When the pool contains multiple copies of the same zstd decoder, it also means the same decoder will be used concurrently by different readers, leading to data corruption and panic.

This is also a security issue and ideally a bugfix release should be published.

Steps to reproduce

diff --git a/config/confighttp/compression_test.go b/config/confighttp/compression_test.go
index 7f7404b09..3d4e85b33 100644
--- a/config/confighttp/compression_test.go
+++ b/config/confighttp/compression_test.go
@@ -613,3 +613,35 @@ func compressLz4(tb testing.TB, body []byte) *bytes.Buffer {
 	require.NoError(tb, lz.Close())
 	return &buf
 }
+
+func TestReproduceZstdPoolingBug(t *testing.T) {
+	h := httpContentDecompressor(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			_, _ = io.ReadAll(r.Body)
+			r.Body.Close()
+		}),
+		defaultMaxRequestBodySize,
+		defaultErrorHandler,
+		defaultCompressionAlgorithms(),
+		availableDecoders,
+	)
+	CONCURRENCY := 10
+	for range CONCURRENCY {
+		go func() {
+			for {
+				payload := compressZstd(t, make([]byte, 2*1024)) // 2KB uncompressed payload
+				assert.NotEmpty(t, payload.Bytes(), "Must have data available")
+
+				req := httptest.NewRequest(http.MethodPost, "/", payload)
+				req.Header.Set("Content-Encoding", "zstd")
+
+				resp := httptest.NewRecorder()
+
+				h.ServeHTTP(resp, req)
+			}
+		}()
+	}
+	// wait forever
+	ch := make(chan struct{})
+	<-ch
+}

What did you expect to see?

no panic

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0xed pc=0xc548df]

goroutine 9 [running]:
github.com/klauspost/compress/zstd.(*Decoder).nextBlockSync(0xc00015c100)
	/home/carson/go/pkg/mod/github.com/klauspost/[email protected]/zstd/decoder.go:531 +0x59f
github.com/klauspost/compress/zstd.(*Decoder).nextBlock(0xc00015c100, 0x10?)
	/home/carson/go/pkg/mod/github.com/klauspost/[email protected]/zstd/decoder.go:421 +0x49
github.com/klauspost/compress/zstd.(*Decoder).Read(0xc00015c100, {0xc0004c8000?, 0x4805c5?, 0x4805a5?})
	/home/carson/go/pkg/mod/github.com/klauspost/[email protected]/zstd/decoder.go:145 +0x134
go.opentelemetry.io/collector/config/confighttp.(*pooledZstdReadCloser).Read(0x41bddf?, {0xc0004c8000?, 0x485269?, 0x200?})
	/home/carson/projects/opentelemetry-collector/config/confighttp/compression.go:56 +0x1b
net/http.(*maxBytesReader).Read(0xc0004c6040, {0xc0004c8000?, 0x41ed54?, 0xc0001c1630?})
	/home/carson/go/pkg/mod/golang.org/[email protected]/src/net/http/request.go:1225 +0x5f
io.ReadAll({0x75fb04200090, 0xc0004c6040})
	/home/carson/go/pkg/mod/golang.org/[email protected]/src/io/io.go:712 +0x75
go.opentelemetry.io/collector/config/confighttp.TestReproduceZstdPoolingBug.func1({0x40?, 0xf5fe00?}, 0xc000b02280)
	/home/carson/projects/opentelemetry-collector/config/confighttp/compression_test.go:620 +0x45
net/http.HandlerFunc.ServeHTTP(0xfee1b9?, {0x1167d10?, 0xc0004c6000?}, 0xe?)
	/home/carson/go/pkg/mod/golang.org/[email protected]/src/net/http/server.go:2322 +0x29
go.opentelemetry.io/collector/config/confighttp.(*decompressor).ServeHTTP(0xc000217e00, {0x1167d10, 0xc0004c6000}, 0xc000b02280)
	/home/carson/projects/opentelemetry-collector/config/confighttp/compression.go:272 +0x204
go.opentelemetry.io/collector/config/confighttp.TestReproduceZstdPoolingBug.func2()
	/home/carson/projects/opentelemetry-collector/config/confighttp/compression_test.go:640 +0x7a
created by go.opentelemetry.io/collector/config/confighttp.TestReproduceZstdPoolingBug in goroutine 7
	/home/carson/projects/opentelemetry-collector/config/confighttp/compression_test.go:630 +0x249

Collector version

0cdc78b

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

OpenTelemetry Collector configuration

Log output

Additional context

No response

Tip

React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it. Learn more here.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions