[cairo-commit] 2 commits - src/cairo-image-info.c

[GitLab Mirror]

 src/cairo-image-info.c |   28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

New commits:
commit 8dbc5893f70b2dde42bdc327f2ab6407394a447d
Merge: 3cc15cb40 e60e562fd
Author: Uli Schlachter <psychon at znc.in>
Date:   Mon Jan 2 09:28:42 2023 +0000

    Merge branch 'jpx-out-of-bounds' into 'master'
    
    Fix possible out-of-bound reads in get_jpx_info
    
    See merge request cairo/cairo!387

commit e60e562fd1eed43f2fc21fd1307004ae09b6e9e5
Author: Uli Schlachter <psychon at znc.in>
Date:   Sun Jan 1 14:01:46 2023 +0100

    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 <psychon at znc.in>

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)