bitbank2 / PNGdec

An optimized PNG decoder suitable for microcontrollers and PCs
Apache License 2.0
172 stars 28 forks source link

The likelihood of an infinite decoding cycle of a damaged file due to the lack of verification of how much data was read in the read callback function #3

Closed Andy-Big closed 2 years ago

Andy-Big commented 2 years ago

Hi, Found your PNG decoding library and now I am trying to embed it into a microcontroller on a 3D printer board. And I found one flaw in the library :) During the decoding process, there is no check for an unexpected end of file. So if the read callback returns 0 or -1, the decoding process continues. The return result of this function is not checked. It turns out that if the file ends unexpectedly (damaged), then the decoding process loops endlessly. Like in this piece of code:

            case 0x49444154: //'IDAT' image data block
                while (iLen) {
                    if (iOffset >= iBytesRead) {
                        // we ran out of data; get some more
                        iBytesRead = (*pPage->pfnRead)(&pPage->PNGFile, pPage->ucFileBuf, (iLen > PNG_FILE_BUF_SIZE) ? PNG_FILE_BUF_SIZE : iLen);
                        iFileOffset += iBytesRead;
                        iOffset = 0;
                    } else {
                        // number of bytes remaining in buffer
                        iBytesRead -= iOffset;
                    }

It would be nice to check the return value by 0 or -1 after calling the read callback function everywhere. If the data is expected to be read and it has not been read, then the file is corrupted or a read error.

            case 0x49444154: //'IDAT' image data block
                while (iLen) {
                    if (iOffset >= iBytesRead) {
                        // we ran out of data; get some more
                        iBytesRead = (*pPage->pfnRead)(&pPage->PNGFile, pPage->ucFileBuf, (iLen > PNG_FILE_BUF_SIZE) ? PNG_FILE_BUF_SIZE : iLen);
                        if (iBytesRead < 1)
                        {
                            pPage->iError = PNG_INVALID_FILE;
                            return pPage->iError;
                        }
                        iFileOffset += iBytesRead;
                        iOffset = 0;
                    } else {
                        // number of bytes remaining in buffer
                        iBytesRead -= iOffset;
                    }
bitbank2 commented 2 years ago

Thanks for finding the error. Please do a pull request for your fix and I'll merge it.