commit d35ce8131530787cff96cf044faf4e4deaf6f9b4
parent 5678c31a0d211d4124dc9b1b3bb73d7aae8c0bbf
Author: Jack Mordaunt <jackmordaunt.dev@gmail.com>
Date: Wed, 16 Sep 2026 21:30:01 -0400
icns: reject malformed elements in the decoder
Element lengths were trusted, so a truncated file panicked with an
out-of-range slice and a zero-length TOC entry underflowed the cursor and
looped forever. Anything decoding untrusted files, the CLI and the Explorer
thumbnail handler included, inherited those crashes. Sentinel errors let
callers tell corruption from unsupported content.
Diffstat:
| M | error.go | | | 17 | +++++++++++++++++ |
| M | reader.go | | | 83 | +++++++++++++++++++++++++++++++++++++++++++------------------------------------ |
| A | reader_test.go | | | 129 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ |
3 files changed, 191 insertions(+), 38 deletions(-)
diff --git a/error.go b/error.go
@@ -1,10 +1,27 @@
package icns
import (
+ "errors"
"fmt"
"image"
)
+// Errors returned by the decoder. They are wrapped with detail, so compare
+// with errors.Is.
+var (
+ // ErrInvalidHeader means the data does not begin with an icns header.
+ ErrInvalidHeader = errors.New("invalid header for icns file")
+ // ErrMalformed means an element's declared length disagrees with the
+ // data present; the file is truncated or corrupt.
+ ErrMalformed = errors.New("malformed icns file")
+ // ErrNoIcons means the file is well formed but contains no icons of a
+ // type this package understands.
+ ErrNoIcons = errors.New("no icons found")
+ // ErrUnsupportedFormat means every icon present uses an image format
+ // this package cannot decode (JPEG 2000).
+ ErrUnsupportedFormat = errors.New("unsupported image format")
+)
+
// ErrImageTooSmall is returned when the image is too small to process.
type ErrImageTooSmall struct {
need int
diff --git a/reader.go b/reader.go
@@ -24,7 +24,7 @@ func Decode(r io.Reader) (image.Image, error) {
})
icon := icons[0]
if icon.IconDescription.ImageFormat == ImageFormatJPEG2000 {
- return nil, fmt.Errorf("decoding largest image (icon %s %s): unsupported format", icon.OsType, icon.ImageFormat)
+ return nil, fmt.Errorf("%w: largest icon %s is %s", ErrUnsupportedFormat, icon.OsType, icon.ImageFormat)
}
img, _, err := image.Decode(icon.r)
if err != nil {
@@ -52,7 +52,7 @@ func DecodeAll(r io.Reader) (images []image.Image, err error) {
images = append(images, img)
}
if len(images) == 0 {
- return nil, fmt.Errorf("no supported icons found")
+ return nil, fmt.Errorf("%w: only %s icons present", ErrUnsupportedFormat, ImageFormatJPEG2000)
}
sort.Slice(images, func(ii, jj int) bool {
var (
@@ -76,54 +76,61 @@ func Probe(r io.Reader) (desc []IconDescription, _ error) {
return desc, nil
}
-// decode identifies the icons in the icns (without decoding the image data).
+// elementHeaderSize is the size of the type and length fields that begin
+// every element, the file header included.
+const elementHeaderSize = 8
+
+// decode identifies the icons in the icns without decoding the image data.
+//
+// An icns file is a sequence of elements, each a 4-byte type followed by a
+// 4-byte big-endian length that counts the whole element, header included.
+// The file itself is one such element of type "icns" enclosing the rest.
+// Every length is validated against the data actually present so malformed
+// or truncated input yields an error rather than a panic or an endless loop.
func decode(r io.Reader) (icons []iconReader, err error) {
data, err := io.ReadAll(r)
if err != nil {
return nil, err
}
- var (
- header = data[0:4]
- fileSize = binary.BigEndian.Uint32(data[4:8])
- read = uint32(8)
- )
- if string(header) != "icns" {
- return nil, fmt.Errorf("invalid header for icns file")
+ if len(data) < elementHeaderSize || string(data[0:4]) != "icns" {
+ return nil, ErrInvalidHeader
}
- for read < fileSize {
- next := data[read : read+4]
- read += 4
- switch string(next) {
- case "TOC ":
- tocSize := binary.BigEndian.Uint32(data[read : read+4])
- read += tocSize - 4 // size includes header and size fields
- continue
- case "icnV":
- read += 4
+ fileSize := int(binary.BigEndian.Uint32(data[4:8]))
+ if fileSize > len(data) {
+ return nil, fmt.Errorf("%w: header declares %d bytes but only %d are present", ErrMalformed, fileSize, len(data))
+ }
+ data = data[:fileSize]
+ for offset := elementHeaderSize; offset < len(data); {
+ if len(data)-offset < elementHeaderSize {
+ return nil, fmt.Errorf("%w: truncated element header at offset %d", ErrMalformed, offset)
+ }
+ var (
+ id = string(data[offset : offset+4])
+ size = int(binary.BigEndian.Uint32(data[offset+4 : offset+8]))
+ )
+ if size < elementHeaderSize || size > len(data)-offset {
+ return nil, fmt.Errorf("%w: element %q at offset %d declares %d bytes", ErrMalformed, id, offset, size)
+ }
+ payload := data[offset+elementHeaderSize : offset+size]
+ offset += size
+ // Elements other than icons ("TOC ", "icnV", "name", "info", ...)
+ // and icons of legacy types carry nothing we can decode; skip them.
+ if !isOsType(id) || len(payload) == 0 {
continue
}
- dataSize := binary.BigEndian.Uint32(data[read : read+4])
- read += 4
- if dataSize == 0 {
- continue // no content, we're not interested
+ ir := iconReader{
+ IconDescription: IconDescription{
+ OsType: osTypeFromID(id),
+ },
+ r: bytes.NewReader(payload),
}
- iconData := data[read : read+dataSize-8]
- read += dataSize - 8 // size includes header and size fields
- if isOsType(string(next)) {
- ir := iconReader{
- IconDescription: IconDescription{
- OsType: osTypeFromID(string(next)),
- },
- r: bytes.NewBuffer(iconData),
- }
- if bytes.Equal(iconData[:8], jpeg2000header) {
- ir.ImageFormat = ImageFormatJPEG2000
- }
- icons = append(icons, ir)
+ if bytes.HasPrefix(payload, jpeg2000header) {
+ ir.ImageFormat = ImageFormatJPEG2000
}
+ icons = append(icons, ir)
}
if len(icons) == 0 {
- return nil, fmt.Errorf("no icons found")
+ return nil, ErrNoIcons
}
return icons, nil
}
diff --git a/reader_test.go b/reader_test.go
@@ -0,0 +1,129 @@
+package icns
+
+import (
+ "bytes"
+ "encoding/binary"
+ "errors"
+ "image"
+ "image/color"
+ "image/png"
+ "testing"
+)
+
+// element builds one icns element: 4-byte type, 4-byte big-endian length of
+// the whole element, then the payload.
+func element(id string, payload []byte) []byte {
+ out := make([]byte, 0, elementHeaderSize+len(payload))
+ out = append(out, id...)
+ out = binary.BigEndian.AppendUint32(out, uint32(elementHeaderSize+len(payload)))
+ return append(out, payload...)
+}
+
+// file wraps elements in an icns header with a correct length.
+func file(elements ...[]byte) []byte {
+ return element("icns", bytes.Join(elements, nil))
+}
+
+func pngBytes(t testing.TB, side int) []byte {
+ t.Helper()
+ var buf bytes.Buffer
+ if err := png.Encode(&buf, image.NewNRGBA(image.Rect(0, 0, side, side))); err != nil {
+ t.Fatal(err)
+ }
+ return buf.Bytes()
+}
+
+func TestDecodeMalformed(t *testing.T) {
+ t.Parallel()
+ tests := []struct {
+ desc string
+ data []byte
+ want error
+ }{
+ {"empty", nil, ErrInvalidHeader},
+ {"short", []byte("ic"), ErrInvalidHeader},
+ {"wrong magic", element("ICNS", nil), ErrInvalidHeader},
+ {"header only", file(), ErrNoIcons},
+ {"declared size exceeds data", append([]byte("icns"), 0xff, 0xff, 0xff, 0xff), ErrMalformed},
+ {"truncated element header", append([]byte("icns\x00\x00\x00\x0c"), 'i', 'c', '0', '7'), ErrMalformed},
+ {"element size below header", file(append([]byte("ic07"), 0, 0, 0, 4)), ErrMalformed},
+ {"element overruns file", file(append([]byte("ic07"), 0, 0, 1, 0)), ErrMalformed},
+ {"zero-length TOC loops forever without a check", file(append([]byte("TOC "), 0, 0, 0, 0)), ErrMalformed},
+ {"unknown elements only", file(element("TOC ", []byte{1, 2, 3, 4}), element("icnV", []byte{0, 0, 0, 0})), ErrNoIcons},
+ {"empty icon payload", file(element("ic07", nil)), ErrNoIcons},
+ {"only jpeg2000", file(element("ic07", jpeg2000header)), ErrUnsupportedFormat},
+ }
+ for _, tt := range tests {
+ t.Run(tt.desc, func(st *testing.T) {
+ _, err := Decode(bytes.NewReader(tt.data))
+ if !errors.Is(err, tt.want) {
+ st.Fatalf("Decode error = %v, want %v", err, tt.want)
+ }
+ if _, err := Probe(bytes.NewReader(tt.data)); err == nil && !errors.Is(tt.want, ErrUnsupportedFormat) {
+ st.Fatalf("Probe accepted data that Decode rejected with %v", tt.want)
+ }
+ })
+ }
+}
+
+func TestDecodeSkipsNonIconElements(t *testing.T) {
+ t.Parallel()
+ data := file(
+ element("TOC ", []byte("ic07\x00\x00\x00\x10")),
+ element("icnV", []byte{0x40, 0x00, 0x00, 0x00}),
+ element("name", []byte("icon")),
+ element("ic07", pngBytes(t, 128)),
+ element("ic11", pngBytes(t, 32)),
+ )
+ desc, err := Probe(bytes.NewReader(data))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(desc) != 2 || desc[0].ID != "ic07" || desc[1].ID != "ic11" {
+ t.Fatalf("Probe = %v, want ic07 and ic11", desc)
+ }
+ // Trailing bytes beyond the declared file size are tolerated.
+ img, err := Decode(bytes.NewReader(append(data, "junk"...)))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got := img.Bounds().Dx(); got != 128 {
+ t.Fatalf("Decode returned a %dpx icon, want 128", got)
+ }
+}
+
+// FuzzDecode checks that arbitrary input never panics or hangs the decoder.
+func FuzzDecode(f *testing.F) {
+ var valid bytes.Buffer
+ if err := Encode(&valid, gradient(64)); err != nil {
+ f.Fatal(err)
+ }
+ f.Add(valid.Bytes())
+ f.Add([]byte{})
+ f.Add(file())
+ f.Add(file(append([]byte("TOC "), 0, 0, 0, 0)))
+ f.Add(file(append([]byte("ic07"), 0, 0, 0, 4)))
+ f.Add(append([]byte("icns"), 0xff, 0xff, 0xff, 0xff))
+ f.Fuzz(func(t *testing.T, data []byte) {
+ Probe(bytes.NewReader(data))
+ Decode(bytes.NewReader(data))
+ DecodeAll(bytes.NewReader(data))
+ })
+}
+
+// gradient returns a side by side image whose colour and alpha vary with
+// position, so an encode/decode roundtrip cannot pass by accident.
+func gradient(side int) *image.NRGBA {
+ img := image.NewNRGBA(image.Rect(0, 0, side, side))
+ for y := 0; y < side; y++ {
+ for x := 0; x < side; x++ {
+ img.SetNRGBA(x, y, color.NRGBA{
+ R: uint8(x * 255 / side),
+ G: uint8(y * 255 / side),
+ B: uint8((x + y) * 255 / (2 * side)),
+ A: uint8(255 - y*255/(2*side)),
+ })
+ }
+ }
+ return img
+}
