github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-09 12:07:47 +00:00
Code Issues Projects Releases Wiki Activity

Remove shared ref to underlying array of JSONFrameReader's Read arg.

Browse Source 
When JSONFrameReader read a JSON object longer than the length of the destination slice, but not
larger than the capacity of the destination slice, it would retain a reference to a slice sharing
the same underlying array as the destination slice. If the underlying array is modified between
calls to Read, corrupt frame data could be returned.

This is also called out in the io.Reader contract: "Implementations must not retain p."
This commit is contained in:
Ben Luddy 2024-02-29 19:21:16 -05:00
parent 055b51728c
commit 10f7a166dd
No known key found for this signature in database
GPG Key ID: A6551E73A5974C30
2 changed files with 27 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
staging/src/k8s.io/apimachinery/pkg/util/framer/framer.go
View File

@ -147,7 +147,6 @@ func (r *jsonFrameReader) Read(data []byte) (int, error) {
// RawMessage#Unmarshal appends to data - we reset the slice down to 0 and will either see
// data written to data, or be larger than data and a different array.
n := len(data)
m := json.RawMessage(data[:0])
if err := r.decoder.Decode(&m); err != nil {
return 0, err
@ -156,12 +155,19 @@ func (r *jsonFrameReader) Read(data []byte) (int, error) {
// If capacity of data is less than length of the message, decoder will allocate a new slice
// and set m to it, which means we need to copy the partial result back into data and preserve
// the remaining result for subsequent reads.
if len(m) > n {
//nolint:staticcheck // SA4006,SA4010 underlying array of data is modified here.
data = append(data[0:0], m[:n]...)
r.remaining = m[n:]
return n, io.ErrShortBuffer
if len(m) > cap(data) {
copy(data, m)
r.remaining = m[len(data):]
return len(data), io.ErrShortBuffer
}
if len(m) > len(data) {
// The bytes beyond len(data) were stored in data's underlying array, which we do
// not own after this function returns.
r.remaining = append([]byte(nil), m[len(data):]...)
return len(data), io.ErrShortBuffer
}
return len(m), nil
}

15
staging/src/k8s.io/apimachinery/pkg/util/framer/framer_test.go
View File

@ -18,6 +18,7 @@ package framer
import (
"bytes"
"errors"
"io"
"testing"
)
@ -173,3 +174,17 @@ func TestJSONFrameReaderShortBuffer(t *testing.T) {
t.Fatalf("unexpected: %v %d %q", err, n, buf)
}
}
func TestJSONFrameReaderShortBufferNoUnderlyingArrayReuse(t *testing.T) {
b := bytes.NewBufferString("{}")
r := NewJSONFramedReader(io.NopCloser(b))
buf := make([]byte, 1, 2) // cap(buf) > len(buf) && cap(buf) <= len("{}")
if n, err := r.Read(buf); !errors.Is(err, io.ErrShortBuffer) || n != 1 || string(buf[:n]) != "{" {
t.Fatalf("unexpected: %v %d %q", err, n, buf)
}
buf = append(buf, make([]byte, 1)...) // stomps the second byte of the backing array
if n, err := r.Read(buf[1:]); err != nil || n != 1 || string(buf[1:1+n]) != "}" {
t.Fatalf("unexpected: %v %d %q", err, n, buf)
}
}