Skip to content

Commit 23ae9ed

Browse files
ZephrFishgopherbot
authored andcommitted
tiff: cap buffer growth to prevent OOM from malicious IFD offset
A crafted 8-byte TIFF file with IFD offset 0xFFFFFFFF causes buffer.fill() to allocate ~4GB of memory when decoding via io.Reader (non-ReaderAt path), leading to an out-of-memory crash in any Go application that calls Decode or DecodeConfig on untrusted input. Read the data, and allocate the buffer, in chunks, to limit memory allocation to the size of the input file. References: https://issuetracker.google.com/issues/494365189 Fixes golang/go#78267 Change-Id: I514161af87fb3ad24180ec4bed61fa49f491e721 GitHub-Last-Rev: 8e6d978 GitHub-Pull-Request: #25 Reviewed-on: https://go-review.googlesource.com/c/image/+/757660 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent e589e60 commit 23ae9ed

2 files changed

Lines changed: 47 additions & 17 deletions

File tree

tiff/buffer.go

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,30 @@
44

55
package tiff
66

7-
import "io"
7+
import (
8+
"io"
9+
"slices"
10+
)
811

912
// buffer buffers an io.Reader to satisfy io.ReaderAt.
1013
type buffer struct {
1114
r io.Reader
1215
buf []byte
1316
}
1417

18+
const fillChunkSize = 10 << 20 // 10 MB
19+
1520
// fill reads data from b.r until the buffer contains at least end bytes.
1621
func (b *buffer) fill(end int) error {
1722
m := len(b.buf)
18-
if end > m {
19-
if end > cap(b.buf) {
20-
newcap := 1024
21-
for newcap < end {
22-
newcap *= 2
23-
}
24-
newbuf := make([]byte, end, newcap)
25-
copy(newbuf, b.buf)
26-
b.buf = newbuf
27-
} else {
28-
b.buf = b.buf[:end]
29-
}
30-
if n, err := io.ReadFull(b.r, b.buf[m:end]); err != nil {
31-
end = m + n
32-
b.buf = b.buf[:end]
23+
for m < end {
24+
next := min(end-m, fillChunkSize)
25+
b.buf = slices.Grow(b.buf, next)
26+
b.buf = b.buf[:m+next]
27+
n, err := io.ReadFull(b.r, b.buf[m:m+next])
28+
m += n
29+
b.buf = b.buf[:m]
30+
if err != nil {
3331
return err
3432
}
3533
}
@@ -44,7 +42,8 @@ func (b *buffer) ReadAt(p []byte, off int64) (int, error) {
4442
}
4543

4644
err := b.fill(end)
47-
return copy(p, b.buf[o:end]), err
45+
end = min(end, len(b.buf))
46+
return copy(p, b.buf[min(o, end):end]), err
4847
}
4948

5049
// Slice returns a slice of the underlying buffer. The slice contains

tiff/reader_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,3 +593,34 @@ func appendIFD(b []byte, enc byteOrder, entries map[uint16]interface{}) []byte {
593593
b = enc.AppendUint32(b, 0)
594594
return b
595595
}
596+
597+
// ioReader wraps an io.Reader to hide any io.ReaderAt implementation,
598+
// forcing the tiff package to use the buffer code path.
599+
type ioReader struct {
600+
io.Reader
601+
}
602+
603+
// TestDecodeOOMIFDOffset tests that a TIFF with an IFD offset of 0xFFFFFFFF
604+
// does not cause an out-of-memory panic in buffer.fill.
605+
func TestDecodeOOMIFDOffset(t *testing.T) {
606+
for _, endian := range []struct {
607+
name string
608+
header []byte
609+
}{
610+
{"little-endian", []byte{'I', 'I', 42, 0, 0xff, 0xff, 0xff, 0xff}},
611+
{"big-endian", []byte{'M', 'M', 0, 42, 0xff, 0xff, 0xff, 0xff}},
612+
} {
613+
t.Run(endian.name, func(t *testing.T) {
614+
r := ioReader{bytes.NewReader(endian.header)}
615+
_, err := Decode(r)
616+
if err == nil {
617+
t.Error("Decode with IFD offset 0xFFFFFFFF: got nil error, want non-nil")
618+
}
619+
r = ioReader{bytes.NewReader(endian.header)}
620+
_, err = DecodeConfig(r)
621+
if err == nil {
622+
t.Error("DecodeConfig with IFD offset 0xFFFFFFFF: got nil error, want non-nil")
623+
}
624+
})
625+
}
626+
}

0 commit comments

Comments
 (0)