diff --git a/CHANGELOG.md b/CHANGELOG.md index cc24dbb5d..87eec434e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ project adheres to [Semantic Versioning](http://semver.org/). ### Changed ### Added ### Fixed +* Use size_t where appropriate and guard against overflow * Fix electron 5 and node 12 compatibility * Fix encoding options (quality) parameter in `canvas.toDataURL()` diff --git a/src/Image.cc b/src/Image.cc index ca2649803..72a32de8c 100644 --- a/src/Image.cc +++ b/src/Image.cc @@ -10,6 +10,11 @@ #include #include "Util.h" +/* Cairo limit: + * https://lists.cairographics.org/archives/cairo/2010-December/021422.html + */ +static constexpr const size_t canvas_max_side = (1 << 15) - 1; + #ifdef HAVE_GIF typedef struct { uint8_t *buf; @@ -252,7 +257,7 @@ NAN_METHOD(Image::SetSource){ argv[0] = Nan::ErrnoException(errorInfo.cerrno, errorInfo.syscall.c_str(), errorInfo.message.c_str(), errorInfo.path.c_str()); } else if (!errorInfo.message.empty()) { argv[0] = Nan::Error(Nan::New(errorInfo.message).ToLocalChecked()); - } else { + } else { argv[0] = Nan::Error(Nan::New(cairo_status_to_string(status)).ToLocalChecked()); } onerrorFn.As()->Call(Isolate::GetCurrent()->GetCurrentContext()->Global(), 1, argv); @@ -453,7 +458,7 @@ Image::loadSurface() { return loadPNG(); } - + if (isGIF(buf)) { #ifdef HAVE_GIF return loadGIF(stream); @@ -607,10 +612,7 @@ Image::loadGIFFromBuffer(uint8_t *buf, unsigned len) { width = naturalWidth = gif->SWidth; height = naturalHeight = gif->SHeight; - /* Cairo limit: - * https://lists.cairographics.org/archives/cairo/2010-December/021422.html - */ - if (width > 32767 || height > 32767) { + if (width > canvas_max_side || height > canvas_max_side) { GIF_CLOSE_FILE(gif); return CAIRO_STATUS_INVALID_SIZE; } @@ -643,9 +645,9 @@ Image::loadGIFFromBuffer(uint8_t *buf, unsigned len) { uint32_t *dst_data = (uint32_t*) data; if (!gif->Image.Interlace) { - if (naturalWidth == img->Width && naturalHeight == img->Height) { - for (int y = 0; y < naturalHeight; ++y) { - for (int x = 0; x < naturalWidth; ++x) { + if (naturalWidth == static_cast(img->Width) && naturalHeight == static_cast(img->Height)) { + for (size_t y = 0; y < naturalHeight; ++y) { + for (size_t x = 0; x < naturalWidth; ++x) { *dst_data = ((*src_data == alphaColor) ? 0 : 255) << 24 | colormap->Colors[*src_data].Red << 16 | colormap->Colors[*src_data].Green << 8 @@ -657,8 +659,8 @@ Image::loadGIFFromBuffer(uint8_t *buf, unsigned len) { } } else { // Image does not take up whole "screen" so we need to fill-in the background - int bottom = img->Top + img->Height; - int right = img->Left + img->Width; + size_t bottom = static_cast(img->Top) + static_cast(img->Height); + size_t right = static_cast(img->Left) + static_cast(img->Width); uint32_t bgPixel = ((bgColor == alphaColor) ? 0 : 255) << 24 @@ -666,9 +668,9 @@ Image::loadGIFFromBuffer(uint8_t *buf, unsigned len) { | colormap->Colors[bgColor].Green << 8 | colormap->Colors[bgColor].Blue; - for (int y = 0; y < naturalHeight; ++y) { - for (int x = 0; x < naturalWidth; ++x) { - if (y < img->Top || y >= bottom || x < img->Left || x >= right) { + for (size_t y = 0; y < naturalHeight; ++y) { + for (size_t x = 0; x < naturalWidth; ++x) { + if (y < static_cast(img->Top) || y >= bottom || x < static_cast(img->Left) || x >= right) { *dst_data = bgPixel; dst_data++; } else { @@ -686,16 +688,16 @@ Image::loadGIFFromBuffer(uint8_t *buf, unsigned len) { // Image is interlaced so that it streams nice over 14.4k and 28.8k modems :) // We first load in 1/8 of the image, followed by another 1/8, followed by // 1/4 and finally the remaining 1/2. - int ioffs[] = { 0, 4, 2, 1 }; - int ijumps[] = { 8, 8, 4, 2 }; + size_t ioffs[] = { 0, 4, 2, 1 }; + size_t ijumps[] = { 8, 8, 4, 2 }; uint8_t *src_ptr = src_data; uint32_t *dst_ptr; - for(int z = 0; z < 4; z++) { - for(int y = ioffs[z]; y < naturalHeight; y += ijumps[z]) { + for(size_t z = 0; z < 4; z++) { + for(size_t y = ioffs[z]; y < naturalHeight; y += ijumps[z]) { dst_ptr = dst_data + naturalWidth * y; - for(int x = 0; x < naturalWidth; ++x) { + for(size_t x = 0; x < naturalWidth; ++x) { *dst_ptr = ((*src_ptr == alphaColor) ? 0 : 255) << 24 | (colormap->Colors[*src_ptr].Red) << 16 | (colormap->Colors[*src_ptr].Green) << 8 @@ -780,12 +782,13 @@ static void jpeg_mem_src (j_decompress_ptr cinfo, void* buffer, long nbytes) { #endif void Image::jpegToARGB(jpeg_decompress_struct* args, uint8_t* data, uint8_t* src, JPEGDecodeL decode) { - int stride = naturalWidth * 4; - for (int y = 0; y < naturalHeight; ++y) { + size_t stride = naturalWidth * 4; + + for (size_t y = 0; y < naturalHeight; ++y) { jpeg_read_scanlines(args, &src, 1); uint32_t *row = (uint32_t*)(data + stride * y); - for (int x = 0; x < naturalWidth; ++x) { - int bx = args->output_components * x; + for (size_t x = 0; x < naturalWidth; ++x) { + size_t bx = args->output_components * x; row[x] = decode(src + bx); } } @@ -799,8 +802,26 @@ void Image::jpegToARGB(jpeg_decompress_struct* args, uint8_t* data, uint8_t* src cairo_status_t Image::decodeJPEGIntoSurface(jpeg_decompress_struct *args) { cairo_status_t status = CAIRO_STATUS_SUCCESS; - - uint8_t *data = new uint8_t[naturalWidth * naturalHeight * 4]; + + size_t stride = naturalWidth * 4; + if (naturalWidth != 0 && stride / naturalWidth != 4) { + // The integer overflowed, which means we cannot allocate that much memory. + jpeg_abort_decompress(args); + jpeg_destroy_decompress(args); + this->errorInfo.set(NULL, "malloc", errno); + return CAIRO_STATUS_NO_MEMORY; + } + + size_t size = naturalHeight * stride; + if (naturalHeight != 0 && size / naturalHeight != stride) { + // The integer overflowed, which means we cannot allocate that much memory. + jpeg_abort_decompress(args); + jpeg_destroy_decompress(args); + this->errorInfo.set(NULL, "malloc", errno); + return CAIRO_STATUS_NO_MEMORY; + } + + uint8_t *data = new uint8_t[size]; if (!data) { jpeg_abort_decompress(args); jpeg_destroy_decompress(args); @@ -1078,6 +1099,12 @@ Image::loadJPEG(FILE *stream) { jpeg_read_header(&args, 1); jpeg_start_decompress(&args); + + if (args.output_width > canvas_max_side || args.output_height > canvas_max_side) { + jpeg_destroy_decompress(&args); + return CAIRO_STATUS_INVALID_SIZE; + } + width = naturalWidth = args.output_width; height = naturalHeight = args.output_height; diff --git a/src/Image.h b/src/Image.h index 62bc3f13b..0f7c92811 100644 --- a/src/Image.h +++ b/src/Image.h @@ -37,8 +37,8 @@ using JPEGDecodeL = std::function; class Image: public Nan::ObjectWrap { public: char *filename; - int width, height; - int naturalWidth, naturalHeight; + size_t width, height; + size_t naturalWidth, naturalHeight; static Nan::Persistent constructor; static void Initialize(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE target); static NAN_METHOD(New); @@ -120,8 +120,8 @@ class Image: public Nan::ObjectWrap { #ifdef HAVE_RSVG RsvgHandle *_rsvg; bool _is_svg; - int _svg_last_width; - int _svg_last_height; + size_t _svg_last_width; + size_t _svg_last_height; #endif ~Image(); };