From fd69c89d05d3205fb237c9603c638668f83c857a Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Fri, 12 Jun 2026 17:10:23 +0800 Subject: [PATCH] fix: complete CVE-2026-39414 S3 Select record limit enforcement Route JSON Lines through the bounded PReader path so oversized records are rejected consistently instead of bypassing the limit on SIMD-capable CPUs. Preserve S3 Select error codes in stream error events, wrap JSON parser errors as JSONParsingError, and flush completed records before returning a terminal error event. Add regression coverage for oversized JSON Lines input and error code preservation. Co-authored-by: Codex Co-authored-by: Claude Code --- internal/s3select/json/preader.go | 2 +- internal/s3select/select.go | 21 ++++--- internal/s3select/select_test.go | 94 +++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 10 deletions(-) diff --git a/internal/s3select/json/preader.go b/internal/s3select/json/preader.go index 64b52d353..9f982893a 100644 --- a/internal/s3select/json/preader.go +++ b/internal/s3select/json/preader.go @@ -244,7 +244,7 @@ func (r *PReader) startReaders() { r.bufferPool.Put(in.input) in.input = nil if err := d.Err(); err != nil { - in.err = err + in.err = errJSONParsingError(err) } in.dst <- all } diff --git a/internal/s3select/select.go b/internal/s3select/select.go index 2aa923774..2bd7bab91 100644 --- a/internal/s3select/select.go +++ b/internal/s3select/select.go @@ -38,10 +38,8 @@ import ( "github.com/minio/minio/internal/s3select/csv" "github.com/minio/minio/internal/s3select/json" "github.com/minio/minio/internal/s3select/parquet" - "github.com/minio/minio/internal/s3select/simdj" "github.com/minio/minio/internal/s3select/sql" "github.com/minio/pkg/v3/env" - "github.com/minio/simdjson-go" "github.com/pierrec/lz4/v4" ) @@ -438,11 +436,8 @@ func (s3Select *S3Select) Open(rsc io.ReadSeekCloser) error { } if strings.EqualFold(s3Select.Input.JSONArgs.ContentType, "lines") { - if simdjson.SupportedCPU() { - s3Select.recordReader = simdj.NewReader(s3Select.progressReader, &s3Select.Input.JSONArgs) - } else { - s3Select.recordReader = json.NewPReader(s3Select.progressReader, &s3Select.Input.JSONArgs) - } + // PReader enforces the S3 Select per-record size limit while splitting lines. + s3Select.recordReader = json.NewPReader(s3Select.progressReader, &s3Select.Input.JSONArgs) } else { // Document mode. s3Select.recordReader = json.NewReader(s3Select.progressReader, &s3Select.Input.JSONArgs) @@ -658,11 +653,19 @@ OuterLoop: } if err != nil { - if serr, ok := err.(SelectError); ok && serr.ErrorCode() == "OverMaxRecordSize" { + selectErr := err + if len(outputQueue) > 0 { + if !sendRecord() { + return + } + } + + var serr SelectError + if errors.As(selectErr, &serr) { _ = writer.FinishWithError(serr.ErrorCode(), serr.ErrorMessage()) return } - _ = writer.FinishWithError("InternalError", err.Error()) + _ = writer.FinishWithError("InternalError", selectErr.Error()) } } diff --git a/internal/s3select/select_test.go b/internal/s3select/select_test.go index 7a623900a..02f4164c7 100644 --- a/internal/s3select/select_test.go +++ b/internal/s3select/select_test.go @@ -70,6 +70,100 @@ func (w *testResponseWriter) WriteHeader(statusCode int) { func (w *testResponseWriter) Flush() { } +func evaluateSelectForTest(t *testing.T, requestXML, input []byte) ([]byte, error) { + t.Helper() + + s3Select, err := NewS3Select(bytes.NewReader(requestXML)) + if err != nil { + t.Fatal(err) + } + if err = s3Select.Open(newBytesRSC(input)); err != nil { + t.Fatal(err) + } + + w := &testResponseWriter{} + s3Select.Evaluate(w) + s3Select.Close() + + resp := http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader(w.response)), + ContentLength: int64(len(w.response)), + } + res, err := minio.NewSelectResults(&resp, "testbucket") + if err != nil { + t.Fatal(err) + } + defer res.Close() + + return io.ReadAll(res) +} + +func TestJSONLinesRejectsOversizedRecord(t *testing.T) { + requestXML := []byte(` + + SELECT id from S3Object s + SQL + + NONE + + LINES + + + + + + + + FALSE + +`) + + input := []byte(`{"id":1}` + "\n" + `{"id":2,"pad":"` + strings.Repeat("a", maxRecordSize) + `"}`) + got, err := evaluateSelectForTest(t, requestXML, input) + if err == nil { + t.Fatal("expected OverMaxRecordSize error") + } + if !strings.Contains(err.Error(), "OverMaxRecordSize") { + t.Fatalf("expected OverMaxRecordSize error, got %v", err) + } + if gotS := strings.TrimSpace(string(got)); gotS != `{"id":1}` { + t.Fatalf("expected records before oversized record to be returned, got %q", gotS) + } +} + +func TestEvaluatePreservesSelectErrorEvents(t *testing.T) { + requestXML := []byte(` + + SELECT * from S3Object + SQL + + NONE + + LINES + + + + + + + + FALSE + +`) + + _, err := evaluateSelectForTest(t, requestXML, []byte("{bad}\n")) + if err == nil { + t.Fatal("expected JSONParsingError") + } + if !strings.Contains(err.Error(), "JSONParsingError") { + t.Fatalf("expected JSONParsingError, got %v", err) + } + if strings.Contains(err.Error(), "InternalError") { + t.Fatalf("select error was folded into InternalError: %v", err) + } +} + func TestJSONQueries(t *testing.T) { input := `{"id": 0,"title": "Test Record","desc": "Some text","synonyms": ["foo", "bar", "whatever"]} {"id": 1,"title": "Second Record","desc": "another text","synonyms": ["some", "synonym", "value"]}