x/image/riff: (*Reader) Next() drops io.ReadFull error #16236

richardlehane · 2016-07-01T03:26:00Z

I'm getting panics and memory issues using golang.org/x/image/riff package with malformed riff files.

E.g. https://play.golang.org/p/cCN1VQ4Dz2

These seem to be due to an IO error being dropped in the (*Reader) Next() method.

From line 132 in https://github.com/golang/image/blob/master/riff/riff.go:

    if _, err = io.ReadFull(z.r, z.buf[:chunkHeaderSize]); err != nil {
        if z.err == io.EOF || z.err == io.ErrUnexpectedEOF {
            z.err = errShortChunkHeader
        }
        return FourCC{}, 0, nil, z.err
    }
    chunkID = FourCC{z.buf[0], z.buf[1], z.buf[2], z.buf[3]}
    z.chunkLen = u32(z.buf[4:])
    z.padded = z.chunkLen&1 == 1
    z.chunkReader = &chunkReader{z}
    return chunkID, z.chunkLen, z.chunkReader, nil

The err var in the first line of that snippet is a named return value but it is never used/ returned by the method.

Can fix by assigning the io.ReadFull error to z.err instead:

    if _, z.err = io.ReadFull(z.r, z.buf[:chunkHeaderSize]); z.err != nil {
        if z.err == io.EOF || z.err == io.ErrUnexpectedEOF {
            z.err = errShortChunkHeader
        }
        return FourCC{}, 0, nil, z.err
    }
    chunkID = FourCC{z.buf[0], z.buf[1], z.buf[2], z.buf[3]}
    z.chunkLen = u32(z.buf[4:])
    z.padded = z.chunkLen&1 == 1
    z.chunkReader = &chunkReader{z}
    return chunkID, z.chunkLen, z.chunkReader, nil

as at golang/image@ 8550bb5

gopherbot · 2016-07-01T07:00:25Z

CL https://golang.org/cl/24638 mentions this issue.

Fixes golang/go#16236 Change-Id: I0e524054d0702a6487ff47d86aed6bf58f4ba3f2 Reviewed-on: https://go-review.googlesource.com/24638 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

bradfitz changed the title ~~image/riff: (*Reader) Next() drops io.ReadFull error~~ x/image/riff: (*Reader) Next() drops io.ReadFull error Jul 1, 2016

bradfitz added this to the Unreleased milestone Jul 1, 2016

bradfitz assigned nigeltao Jul 1, 2016

gopherbot closed this as completed in golang/image@a21e5be Jul 7, 2016

golang locked and limited conversation to collaborators Jul 7, 2017

gopherbot added the FrozenDueToAge label Jul 7, 2017

rsc unassigned nigeltao Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/image/riff: (*Reader) Next() drops io.ReadFull error #16236

x/image/riff: (*Reader) Next() drops io.ReadFull error #16236

richardlehane commented Jul 1, 2016

gopherbot commented Jul 1, 2016

x/image/riff: (*Reader) Next() drops io.ReadFull error #16236

x/image/riff: (*Reader) Next() drops io.ReadFull error #16236

Comments

richardlehane commented Jul 1, 2016

gopherbot commented Jul 1, 2016