From d515dbdfeba0f3c02deb17dce5ca1f958fc0befb Mon Sep 17 00:00:00 2001 From: Lode Date: Thu, 27 Nov 2014 10:46:43 +0100 Subject: various fixes --- example_png_info.cpp | 12 ++++++++---- lodepng.cpp | 44 ++++++++++++++++++++++++++++++-------------- lodepng.h | 2 +- lodepng_unittest.cpp | 6 +++--- lodepng_util.cpp | 14 ++++++++++---- 5 files changed, 52 insertions(+), 26 deletions(-) diff --git a/example_png_info.cpp b/example_png_info.cpp index afd845c..d9112b6 100644 --- a/example_png_info.cpp +++ b/example_png_info.cpp @@ -100,7 +100,7 @@ Display the names and sizes of all chunks in the PNG file. void displayChunkNames(const std::vector& buffer) { // Listing chunks is based on the original file, not the decoded png info. - const unsigned char *chunk, *begin, *end; + const unsigned char *chunk, *begin, *end, *next; end = &buffer.back() + 1; begin = chunk = &buffer.front() + 8; @@ -126,7 +126,9 @@ void displayChunkNames(const std::vector& buffer) std::cout << lodepng_chunk_length(chunk) << ", "; - chunk = lodepng_chunk_next_const(chunk); + next = lodepng_chunk_next_const(chunk); + if (next <= chunk) break; // integer overflow + chunk = next; } std::cout << std::endl; } @@ -212,7 +214,7 @@ void displayFilterTypes(const std::vector& buffer) } //Read literal data from all IDAT chunks - const unsigned char *chunk, *begin, *end; + const unsigned char *chunk, *begin, *end, *next; end = &buffer.back() + 1; begin = chunk = &buffer.front() + 8; @@ -243,7 +245,9 @@ void displayFilterTypes(const std::vector& buffer) } } - chunk = lodepng_chunk_next_const(chunk); + next = lodepng_chunk_next_const(chunk); + if (next <= chunk) break; // integer overflow + chunk = next; } //Decompress all IDAT data diff --git a/lodepng.cpp b/lodepng.cpp index e1d6dbe..d7486f3 100644 --- a/lodepng.cpp +++ b/lodepng.cpp @@ -1,5 +1,5 @@ /* -LodePNG version 20141120 +LodePNG version 20141126 Copyright (c) 2005-2014 Lode Vandevenne @@ -37,7 +37,7 @@ Rename this file to lodepng.cpp to use it for C++, or to lodepng.c to use it for #include #endif /*LODEPNG_COMPILE_CPP*/ -#define VERSION_STRING "20141120" +#define VERSION_STRING "20141126" #if defined(_MSC_VER) && (_MSC_VER >= 1310) /*Visual Studio: A few warning types are not desired here.*/ #pragma warning( disable : 4244 ) /*implicit conversions: not warned by gcc -Wall -Wextra and requires too much casts*/ @@ -119,6 +119,13 @@ Example: if(!uivector_resizev(&frequencies_ll, 286, 0)) ERROR_BREAK(83); if(error) return error;\ } +/*Set error var to the error code, and return from the void function.*/ +#define CERROR_RETURN(errorvar, code)\ +{\ + errorvar = code;\ + return;\ +} + /* About uivector, ucvector and string: -All of them wrap dynamic arrays or text strings in a similar way. @@ -566,7 +573,8 @@ static unsigned HuffmanTree_make2DTree(HuffmanTree* tree) for(i = 0; i < tree->lengths[n]; i++) /*the bits for this code*/ { unsigned char bit = (unsigned char)((tree->tree1d[n] >> (tree->lengths[n] - i - 1)) & 1); - if(treepos > tree->numcodes - 2) return 55; /*oversubscribed, see comment in lodepng_error_text*/ + /*oversubscribed, see comment in lodepng_error_text*/ + if(treepos > 2147483647 || treepos + 2 > tree->numcodes) return 55; if(tree->tree2d[2 * treepos + bit] == 32767) /*not yet filled in*/ { if(i + 1 == tree->lengths[n]) /*last bit*/ @@ -831,7 +839,7 @@ unsigned lodepng_huffman_code_lengths(unsigned* lengths, const unsigned* frequen if(!error) { /*calculate the lenghts of each symbol, as the amount of times a coin of each symbol is used*/ - for(i = 0; i < numpresent - 1; i++) + for(i = 0; i + 1 < numpresent; i++) { Coin* coin = &coins[i]; for(j = 0; j < coin->symbols.size; j++) lengths[coin->symbols.data[j]]++; @@ -1194,14 +1202,15 @@ static unsigned inflateHuffmanBlock(ucvector* out, const unsigned char* in, size static unsigned inflateNoCompression(ucvector* out, const unsigned char* in, size_t* bp, size_t* pos, size_t inlength) { - /*go to first boundary of byte*/ size_t p; unsigned LEN, NLEN, n, error = 0; + + /*go to first boundary of byte*/ while(((*bp) & 0x7) != 0) (*bp)++; p = (*bp) / 8; /*byte position*/ /*read LEN (2 bytes) and NLEN (2 bytes)*/ - if(p >= inlength - 4) return 52; /*error, bit pointer will jump past memory*/ + if(p + 4 >= inlength) return 52; /*error, bit pointer will jump past memory*/ LEN = in[p] + 256u * in[p + 1]; p += 2; NLEN = in[p] + 256u * in[p + 1]; p += 2; @@ -1541,7 +1550,7 @@ static unsigned encodeLZ77(uivector* out, Hash* hash, } if(hashpos == hash->chain[hashpos]) break; - + if(numzeros >= 3 && length > numzeros) { hashpos = hash->chainz[hashpos]; if(hash->zeros[hashpos] != numzeros) break; @@ -3548,7 +3557,7 @@ unsigned lodepng_get_color_profile(LodePNGColorProfile* profile, for(i = 0; i < numpixels; i++) { getPixelColorRGBA16(&r, &g, &b, &a, in, i, mode); - + if(!colored_done && (r != g || r != b)) { profile->colored = 1; @@ -3813,7 +3822,7 @@ unsigned lodepng_inspect(unsigned* w, unsigned* h, LodePNGState* state, { CERROR_RETURN_ERROR(state->error, 48); /*error: the given data is empty*/ } - if(insize < 29) + if(insize < 33) { CERROR_RETURN_ERROR(state->error, 27); /*error: the data length is smaller than the length of a PNG header*/ } @@ -3841,6 +3850,11 @@ unsigned lodepng_inspect(unsigned* w, unsigned* h, LodePNGState* state, info->filter_method = in[27]; info->interlace_method = in[28]; + if(*w == 0 || *h == 0) + { + CERROR_RETURN_ERROR(state->error, 93); + } + if(!state->decoder.ignore_crc) { unsigned CRC = lodepng_read32bitInt(&in[29]); @@ -4440,11 +4454,12 @@ static void decodeGeneric(unsigned char** out, unsigned* w, unsigned* h, if(state->error) return; numpixels = *w * *h; - if(*h != 0 && numpixels / *h != *w) - { - state->error = 92; /*multiplication overflow*/ - return; - } + + /*multiplication overflow*/ + if(*h != 0 && numpixels / *h != *w) CERROR_RETURN(state->error, 92); + /*multiplication overflow possible further below. Allows up to 2^31-1 pixel + bytes with 16-bit RGBA, the rest is room for filter bytes.*/ + if(numpixels > 268435455) CERROR_RETURN(state->error, 92); ucvector_init(&idat); chunk = &in[33]; /*first byte of the first chunk after the header*/ @@ -5889,6 +5904,7 @@ const char* lodepng_error_text(unsigned code) case 90: return "windowsize must be a power of two"; case 91: return "invalid decompressed idat size"; case 92: return "too many pixels, not supported"; + case 93: return "zero width or height is invalid"; } return "unknown error code"; } diff --git a/lodepng.h b/lodepng.h index b131ef3..b9e90c4 100644 --- a/lodepng.h +++ b/lodepng.h @@ -1,5 +1,5 @@ /* -LodePNG version 20141120 +LodePNG version 20141126 Copyright (c) 2005-2014 Lode Vandevenne diff --git a/lodepng_unittest.cpp b/lodepng_unittest.cpp index e3dbc10..56fc61e 100644 --- a/lodepng_unittest.cpp +++ b/lodepng_unittest.cpp @@ -43,7 +43,7 @@ mv lodepng.cpp lodepng.c ; gcc lodepng.c example*.c -W -Wall -ansi -pedantic -O3 *) Check pngdetail.cpp: g++ lodepng.cpp lodepng_util.cpp pngdetail.cpp -W -Wall -ansi -pedantic -O3 -o pngdetail -./pnginfo testdata/PngSuite/basi0g01.png +./pngdetail testdata/PngSuite/basi0g01.png *) Test compiling with some code sections with #defines disabled, for unused static function warnings etc... g++ lodepng.cpp -W -Wall -ansi -pedantic -O3 -c -DLODEPNG_NO_COMPILE_ZLIB @@ -69,7 +69,7 @@ clang++ --analyze -Xanalyzer -analyzer-output=html lodepng.cpp *) check for memory leaks and vulnerabilities with valgrind comment out the large files tests because they're slow with valgrind -g++ lodepng.cpp lodepng_util.cpp lodepng_unittest.cpp -Wall -Wextra -pedantic -ansi -g3 +g++ lodepng.cpp lodepng_util.cpp lodepng_unittest.cpp -Wall -Wextra -pedantic -ansi -O3 valgrind --leak-check=full --track-origins=yes ./a.out *) remove "#include " from lodepng.cpp if it's still in there @@ -85,7 +85,7 @@ g++ lodepng.cpp example_sdl.cpp -Wall -Wextra -pedantic -ansi -O3 -lSDL -o showp // ./showpng testdata/PngSuite/*.png /* -*) strip trailing spaces +*) strip trailing spaces and ensure consistent newlines *) check diff of lodepng.cpp and lodepng.h before submitting diff --git a/lodepng_util.cpp b/lodepng_util.cpp index 37a6e73..3e59a52 100644 --- a/lodepng_util.cpp +++ b/lodepng_util.cpp @@ -41,7 +41,7 @@ unsigned getChunkInfo(std::vector& names, std::vector& size const std::vector& png) { // Listing chunks is based on the original file, not the decoded png info. - const unsigned char *chunk, *begin, *end; + const unsigned char *chunk, *begin, *end, *next; end = &png.back() + 1; begin = chunk = &png.front() + 8; @@ -56,7 +56,9 @@ unsigned getChunkInfo(std::vector& names, std::vector& size names.push_back(type); sizes.push_back(length); - chunk = lodepng_chunk_next_const(chunk); + next = lodepng_chunk_next_const(chunk); + if (next <= chunk) return 1; // integer overflow + chunk = next; } return 0; } @@ -79,6 +81,7 @@ unsigned getChunks(std::vector names[3], if(name.size() != 4) return 1; next = lodepng_chunk_next_const(chunk); + if (next <= chunk) return 1; // integer overflow if(name == "IHDR") { @@ -123,6 +126,7 @@ unsigned insertChunks(std::vector& png, if(name.size() != 4) return 1; next = lodepng_chunk_next_const(chunk); + if (next <= chunk) return 1; // integer overflow if(name == "PLTE") { @@ -166,7 +170,7 @@ unsigned getFilterTypesInterlaced(std::vector >& filt if(error) return 1; //Read literal data from all IDAT chunks - const unsigned char *chunk, *begin, *end; + const unsigned char *chunk, *begin, *end, *next; end = &png.back() + 1; begin = chunk = &png.front() + 8; @@ -190,7 +194,9 @@ unsigned getFilterTypesInterlaced(std::vector >& filt } } - chunk = lodepng_chunk_next_const(chunk); + next = lodepng_chunk_next_const(chunk); + if (next <= chunk) return 1; // integer overflow + chunk = next; } //Decompress all IDAT data -- cgit v1.2.3