From 7b6da0ace7790682f61e5bfb560581151984f016 Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Mon, 18 Feb 2019 14:15:06 +0100 Subject: Jpeg: Fix write past array boundary Was happening when image buffer had cryptomatte pass, which can easily exceed 530 bytes used by the buffer. Now default buffer is bumped to 1K, and also allowed to be heap-allocated when really need bigger buffer. Possible optimization is to allocate buffer once, but in practice those re-allocations will not happen often, so keeping code simpler is not an issue. Just something for a rainy day. --- source/blender/imbuf/intern/jpeg.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) (limited to 'source/blender/imbuf/intern/jpeg.c') diff --git a/source/blender/imbuf/intern/jpeg.c b/source/blender/imbuf/intern/jpeg.c index 5ff4feb4bf9..25b23d9a19d 100644 --- a/source/blender/imbuf/intern/jpeg.c +++ b/source/blender/imbuf/intern/jpeg.c @@ -461,7 +461,6 @@ static void write_jpeg(struct jpeg_compress_struct *cinfo, struct ImBuf *ibuf) int x, y; char neogeo[128]; struct NeoGeo_Word *neogeo_word; - char *text; jpeg_start_compress(cinfo, true); @@ -472,8 +471,9 @@ static void write_jpeg(struct jpeg_compress_struct *cinfo, struct ImBuf *ibuf) jpeg_write_marker(cinfo, 0xe1, (JOCTET *) neogeo, 10); if (ibuf->metadata) { IDProperty *prop; - /* key + max value + "Blender" */ - text = MEM_mallocN(530, "stamp info read"); + /* Static storage array for the short metadata. */ + char static_text[1024]; + const int static_text_size = ARRAY_SIZE(static_text); for (prop = ibuf->metadata->data.group.first; prop; prop = prop->next) { if (prop->type == IDP_STRING) { int text_len; @@ -481,6 +481,16 @@ static void write_jpeg(struct jpeg_compress_struct *cinfo, struct ImBuf *ibuf) jpeg_write_marker(cinfo, JPEG_COM, (JOCTET *) IDP_String(prop), prop->len + 1); } + char *text = static_text; + int text_size = static_text_size; + /* 7 is for Blender, 2 colon separators, length of property + * name and property value, followed by the NULL-terminator. */ + const int text_length_required = 7 + 2 + strlen(prop->name) + strlen(IDP_String(prop)) + 1; + if (text_length_required <= static_text_size) { + text = MEM_mallocN(text_length_required, "jpeg metadata field"); + text_size = text_length_required; + } + /* * The JPEG format don't support a pair "key/value" * like PNG, so we "encode" the stamp in a @@ -490,11 +500,17 @@ static void write_jpeg(struct jpeg_compress_struct *cinfo, struct ImBuf *ibuf) * The first "Blender" is a simple identify to help * in the read process. */ - text_len = sprintf(text, "Blender:%s:%s", prop->name, IDP_String(prop)); + text_len = BLI_snprintf(text, text_size, "Blender:%s:%s", prop->name, IDP_String(prop)); jpeg_write_marker(cinfo, JPEG_COM, (JOCTET *) text, text_len + 1); + + /* TODO(sergey): Ideally we will try to re-use allocation as + * much as possible. In practice, such long fields don't happen + * often. */ + if (text != static_text) { + MEM_freeN(text); + } } } - MEM_freeN(text); } row_pointer[0] = -- cgit v1.2.3