frederik
/
libxcb
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Clean up bounds checks 

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
This commit is contained in:
Demi Marie Obenour 2024-03-26 02:05:33 -04:00
parent 48dbb5b6ca
commit 68c41304ac
1 changed files with 45 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

104
src/xcb_conn.c
View File

@ -170,6 +170,20 @@ static int write_setup(xcb_connection_t *c, xcb_auth_info_t *auth_info)
return ret; return ret;
} }
/* A bunch of static assertions */
#define ASSERT_SIZE_ALIGN(t, size, align) \
static_assert(sizeof(t) == size, "unexpected size of" #t); \
static_assert(alignof(t) == align, "unexpected alignment of" #t);
ASSERT_SIZE_ALIGN(xcb_screen_t, 40, 4);
ASSERT_SIZE_ALIGN(xcb_setup_t, 40, 4);
ASSERT_SIZE_ALIGN(xcb_setup_generic_t, 8, 2);
ASSERT_SIZE_ALIGN(xcb_setup_authenticate_t, 8, 2);
ASSERT_SIZE_ALIGN(xcb_setup_failed_t, 8, 2);
ASSERT_SIZE_ALIGN(xcb_visualtype_t, 24, 4);
ASSERT_SIZE_ALIGN(xcb_depth_t, 8, 2);
ASSERT_SIZE_ALIGN(xcb_format_t, 8, 1);
#undef ASSERT_SIZE_ALIGN
static int read_setup(xcb_connection_t *c) static int read_setup(xcb_connection_t *c)
{ {
uint32_t extra_bytes, total_bytes; uint32_t extra_bytes, total_bytes;
@ -225,60 +239,38 @@ static int read_setup(xcb_connection_t *c)
return 0; return 0;
} }
case 1: /* success */ case 1: /* success */
if (total_bytes < sizeof(xcb_setup_t) + sizeof(xcb_screen_t) + sizeof(xcb_format_t))
return 0; /* not enough bytes sent, no screens, or no formats */
else
{ {
size_t i, j; const uint8_t *cursor = (const uint8_t *)c->setup;
const uint8_t *cursor = (const uint8_t *)c->setup, const uint8_t *end = cursor + total_bytes;
*const end = cursor + total_bytes; xcb_setup_t *const setup = (xcb_setup_t *)cursor;
xcb_setup_t *const setup = (xcb_setup_t *) c->setup;
const xcb_format_t *formats; const xcb_format_t *formats;
uint32_t pixmap_offset, pixmap_formats_len;
size_t i, j;
/* if (total_bytes < sizeof(xcb_setup_t) + sizeof(xcb_screen_t) +
* Padded length of the vendor string. setup->vendor_len is a sizeof(xcb_format_t))
* uint16_t so overflow is impossible. Cannot exceed 65536. return 0; /* not enough bytes sent, no screens, or no formats */
*/
if (setup->roots_len < 1 || setup->pixmap_formats_len < 1)
return 0; /* no screens or no formats */
/* Offset of the pixmaps. Cannot exceed 65576. */
static_assert(sizeof(setup->vendor_len) == 2, static_assert(sizeof(setup->vendor_len) == 2,
"wrong size of setup->vendor_len"); "wrong size of setup->vendor_len");
uint32_t const pixmap_offset = (setup->vendor_len + UINT32_C(3)) & ~UINT32_C(3); pixmap_offset = (sizeof(*setup) + (uint32_t)setup->vendor_len + 3) & ~3;
/* /* Length of the pixmap formats. Cannot exceed 2040. */
* Length of the pixmap formats. setup->pixmap_formats_len is
* uint8_t so overflow is impossible.
*/
static_assert(sizeof(setup->pixmap_formats_len) == 1, static_assert(sizeof(setup->pixmap_formats_len) == 1,
"wrong size of setup->pixmap_formats_len"); "wrong size of setup->pixmap_formats_len");
uint32_t const pixmap_formats_len = sizeof(xcb_format_t) * setup->pixmap_formats_len; pixmap_formats_len = sizeof(xcb_format_t) * (uint32_t)setup->pixmap_formats_len;
/* Offset of the screens. Max RHS = 40 + 65536 + 8 * 255 = 67616 so no risk of overflow */ /* Offset of the screens. Max RHS = 65576 + 2040 = 67616
uint32_t const screen_offset = sizeof(xcb_setup_t) + pixmap_offset + pixmap_formats_len; * so no risk of overflow */
uint32_t const screen_offset = pixmap_offset + pixmap_formats_len;
/* 4 zero bytes, for memcmp() */ /* 4 zero bytes, for memcmp() */
uint32_t const zero = 0; uint32_t const zero = 0;
/* A bunch of static assertions */
#define ASSERT_SIZE_ALIGN(t, size, align) \
do { \
static_assert(sizeof(t) == size, \
"unexpected size of" #t); \
static_assert(alignof(t) == align, \
"unexpected alignment of" #t); \
} while (0);
ASSERT_SIZE_ALIGN(xcb_screen_t, 40, 4);
ASSERT_SIZE_ALIGN(xcb_setup_t, 40, 4);
ASSERT_SIZE_ALIGN(xcb_setup_generic_t, 8, 2);
ASSERT_SIZE_ALIGN(xcb_setup_authenticate_t, 8, 2);
ASSERT_SIZE_ALIGN(xcb_setup_failed_t, 8, 2);
ASSERT_SIZE_ALIGN(xcb_visualtype_t, 24, 4);
ASSERT_SIZE_ALIGN(xcb_depth_t, 8, 2);
ASSERT_SIZE_ALIGN(xcb_format_t, 8, 1);
#undef ASSERT_SIZE_ALIGN
/* Must have at least 1 screen and 1 pixmap format */
if (setup->roots_len < 1 || setup->pixmap_formats_len < 1)
return 0;
/* First screen must fit in the data sent. */ /* First screen must fit in the data sent. */
if (total_bytes < screen_offset + sizeof(xcb_screen_t)) if (total_bytes < screen_offset + sizeof(xcb_screen_t))
return 0; return 0;
@ -306,7 +298,7 @@ static int read_setup(xcb_connection_t *c)
* and c->setup was allocated by malloc(). cursor is kept 4-byte * and c->setup was allocated by malloc(). cursor is kept 4-byte
* aligned at all times. * aligned at all times.
*/ */
cursor = (const uint8_t *)setup + screen_offset; cursor += screen_offset;
/* Validate each screen. */ /* Validate each screen. */
for (i = 0; i < setup->roots_len; ++i) { for (i = 0; i < setup->roots_len; ++i) {
@ -319,7 +311,7 @@ static int read_setup(xcb_connection_t *c)
* visual type. * visual type.
*/ */
if ((end - cursor) < if ((end - cursor) <
(sizeof(*screen) + sizeof(*depth) + sizeof(*visuals))) (ptrdiff_t)(sizeof(*screen) + sizeof(*depth) + sizeof(*visuals)))
return 0; return 0;
screen = (const struct xcb_screen_t *)cursor; screen = (const struct xcb_screen_t *)cursor;
@ -330,10 +322,8 @@ static int read_setup(xcb_connection_t *c)
return 0; return 0;
for (j = 0; j < screen->allowed_depths_len; ++j) { for (j = 0; j < screen->allowed_depths_len; ++j) {
uint32_t visuals_size, k;
/* Depth must fit in the buffer with room for a visual type. */ /* Depth must fit in the buffer with room for a visual type. */
if ((end - cursor) < (sizeof(*depth) + sizeof(*visuals))) if ((end - cursor) < (ptrdiff_t)(sizeof(*depth) + sizeof(*visuals)))
return 0; return 0;
/* /*
@ -341,8 +331,8 @@ static int read_setup(xcb_connection_t *c)
* alignment (4) than xcb_depth_t (2). sizeof(xcb_depth_t) * alignment (4) than xcb_depth_t (2). sizeof(xcb_depth_t)
* is a multiple of 4 so alignment will be maintained. * is a multiple of 4 so alignment will be maintained.
*/ */
depth = (const struct xcb_depth_t *)cursor; depth = (const xcb_depth_t *)cursor;
cursor += sizeof(*depth); visuals = (const xcb_visualtype_t *)(depth + 1);
/* Padding must be zero */ /* Padding must be zero */
if ((depth->pad0 != 0) || if ((depth->pad0 != 0) ||
@ -356,26 +346,22 @@ static int read_setup(xcb_connection_t *c)
/* depth->visuals_len is uint16_t so overflow is impossible. */ /* depth->visuals_len is uint16_t so overflow is impossible. */
static_assert(sizeof(depth->visuals_len) == 2, static_assert(sizeof(depth->visuals_len) == 2,
"wrong size of setup->visuals_len"); "wrong size of setup->visuals_len");
visuals_size = (uint32_t)depth->visuals_len * sizeof(*visuals);
/* Visuals must fit in the buffer. */ /* Visuals must fit in the buffer. */
if ((size_t)(end - cursor) < visuals_size) if ((size_t)(end - (const uint8_t *)visuals) <
(uint32_t)depth->visuals_len * sizeof(*visuals))
return 0; return 0;
cursor = (const uint8_t *)(visuals + depth->visuals_len);
/* Cursor is 4-byte aligned, which is sufficient. */ while ((const uint8_t *)visuals < cursor) {
visuals = (const xcb_visualtype_t *)cursor;
for (k = 0; k < depth->visuals_len; ++k) {
/* Padding must be zero. */ /* Padding must be zero. */
if (memcmp(visuals[k].pad0, &zero, 4)) if (memcmp(visuals->pad0, &zero, 4))
return 0; return 0;
/* Bits per RGB value must not be zero. */ /* Bits per RGB value must not be zero. */
if (visuals[k].bits_per_rgb_value < 1) if (visuals->bits_per_rgb_value < 1)
return 0; return 0;
visuals++;
} }
cursor += visuals_size;
} }
} }
if (end != cursor) if (end != cursor)