From e60e562fd1eed43f2fc21fd1307004ae09b6e9e5 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 1 Jan 2023 14:01:46 +0100 Subject: [PATCH] Fix possible out-of-bound reads in get_jpx_info Inspired by [1], I looked into the other functions in cairo-image-info.c. This commit fixes the possible out-of-bound reads that I found just by staring at the code. _jpx_next_box() would happily read beyond the end of the data via get_unaligned_be32(). This commit adds checks that at least for bytes of data are available. Additionally, I made this function check that its returned pointer is within bounds, just because I found this easier to reason about. Also, _jpx_extract_info() did not check that it had enough data to read. This is fixed by making the function fallible and giving it information about the end of data. [1]: https://gitlab.freedesktop.org/cairo/cairo/-/merge_requests/386 Signed-off-by: Uli Schlachter --- src/cairo-image-info.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/cairo-image-info.c b/src/cairo-image-info.c index f207ae887..a2403ef3f 100644 --- a/src/cairo-image-info.c +++ b/src/cairo-image-info.c @@ -162,9 +162,15 @@ static const unsigned char _jpx_signature[] = { }; static const unsigned char * -_jpx_next_box (const unsigned char *p) +_jpx_next_box (const unsigned char *p, const unsigned char *end) { - return p + get_unaligned_be32 (p); + if (p + 4 < end) { + uint32_t length = get_unaligned_be32 (p); + if (p + length < end) + return p + length; + } + + return end; } static const unsigned char * @@ -193,19 +199,25 @@ _jpx_find_box (const unsigned char *p, const unsigned char *end, uint32_t type) while (p < end) { if (_jpx_match_box (p, end, type)) return p; - p = _jpx_next_box (p); + p = _jpx_next_box (p, end); } return NULL; } -static void -_jpx_extract_info (const unsigned char *p, cairo_image_info_t *info) +static cairo_int_status_t +_jpx_extract_info (const unsigned char *p, cairo_image_info_t *info, const unsigned char *end) { + if (p + 11 >= end) { + return CAIRO_INT_STATUS_UNSUPPORTED; + } + info->height = get_unaligned_be32 (p); info->width = get_unaligned_be32 (p + 4); info->num_components = (p[8] << 8) + p[9]; info->bits_per_component = p[10]; + + return CAIRO_STATUS_SUCCESS; } cairo_int_status_t @@ -227,7 +239,7 @@ _cairo_image_info_get_jpx_info (cairo_image_info_t *info, if (! _jpx_match_box (p, end, JPX_FILETYPE)) return CAIRO_INT_STATUS_UNSUPPORTED; - p = _jpx_next_box (p); + p = _jpx_next_box (p, end); /* Locate the JP2 header box. */ p = _jpx_find_box (p, end, JPX_JP2_HEADER); @@ -242,9 +254,7 @@ _cairo_image_info_get_jpx_info (cairo_image_info_t *info, /* Get the image info */ p = _jpx_get_box_contents (p); - _jpx_extract_info (p, info); - - return CAIRO_STATUS_SUCCESS; + return _jpx_extract_info (p, info, end); } /* PNG (image/png)