mirrors/wayland
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/wayland/wayland.git synced 2025-10-29 05:40:16 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

connection: Prevent integer overflow in DIV_ROUNDUP.

Browse source 
The DIV_ROUNDUP macro would overflow when trying to round values higher
than MAX_UINT32 - (a - 1). The result is 0 after the division. This is
potential security issue when demarshalling an array because the length
check is performed with the overflowed value, but then the original huge
value is stored for later use.

The issue was present only on 32bit platforms. The use of size_t in the
DIV_ROUNDUP macro already promoted everything to 64 bit size on 64 bit
systems.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

Style changes by Derek Foreman
This commit is contained in:
Michal Srb 2018-08-14 13:07:52 +02:00 • committed by Derek Foreman
parent fde60465c3
commit f5b9e3b9a1
1 changed files with 17 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

26
src/connection.c
View file

@ -44,7 +44,15 @@
#include "wayland-private.h" #include "wayland-private.h"
#include "wayland-os.h" #include "wayland-os.h"
#define DIV_ROUNDUP(n, a) ( ((n) + ((a) - 1)) / (a) ) static inline uint32_t
div_roundup(uint32_t n, size_t a)
{
/* The cast to uint64_t is necessary to prevent overflow when rounding
* values close to UINT32_MAX. After the division it is again safe to
* cast back to uint32_t.
*/
return (uint32_t) (((uint64_t) n + (a - 1)) / a);
}
struct wl_buffer { struct wl_buffer {
char data[4096]; char data[4096];
@ -734,7 +742,7 @@ wl_connection_demarshal(struct wl_connection *connection,
break; break;
} }
next = p + DIV_ROUNDUP(length, sizeof *p); next = p + div_roundup(length, sizeof *p);
if (next > end) { if (next > end) {
wl_log("message too short, " wl_log("message too short, "
"object (%d), message %s(%s)\n", "object (%d), message %s(%s)\n",
@ -793,7 +801,7 @@ wl_connection_demarshal(struct wl_connection *connection,
case 'a': case 'a':
length = *p++; length = *p++;
next = p + DIV_ROUNDUP(length, sizeof *p); next = p + div_roundup(length, sizeof *p);
if (next > end) { if (next > end) {
wl_log("message too short, " wl_log("message too short, "
"object (%d), message %s(%s)\n", "object (%d), message %s(%s)\n",
@ -1068,7 +1076,7 @@ buffer_size_for_closure(struct wl_closure *closure)
} }
size = strlen(closure->args[i].s) + 1; size = strlen(closure->args[i].s) + 1;
buffer_size += 1 + DIV_ROUNDUP(size, sizeof(uint32_t)); buffer_size += 1 + div_roundup(size, sizeof(uint32_t));
break; break;
case 'a': case 'a':
if (closure->args[i].a == NULL) { if (closure->args[i].a == NULL) {
@ -1077,7 +1085,7 @@ buffer_size_for_closure(struct wl_closure *closure)
} }
size = closure->args[i].a->size; size = closure->args[i].a->size;
buffer_size += (1 + DIV_ROUNDUP(size, sizeof(uint32_t))); buffer_size += (1 + div_roundup(size, sizeof(uint32_t)));
break; break;
default: default:
break; break;
@ -1139,11 +1147,11 @@ serialize_closure(struct wl_closure *closure, uint32_t *buffer,
size = strlen(closure->args[i].s) + 1; size = strlen(closure->args[i].s) + 1;
*p++ = size; *p++ = size;
if (p + DIV_ROUNDUP(size, sizeof *p) > end) if (p + div_roundup(size, sizeof *p) > end)
goto overflow; goto overflow;
memcpy(p, closure->args[i].s, size); memcpy(p, closure->args[i].s, size);
p += DIV_ROUNDUP(size, sizeof *p); p += div_roundup(size, sizeof *p);
break; break;
case 'a': case 'a':
if (closure->args[i].a == NULL) { if (closure->args[i].a == NULL) {
@ -1154,11 +1162,11 @@ serialize_closure(struct wl_closure *closure, uint32_t *buffer,
size = closure->args[i].a->size; size = closure->args[i].a->size;
*p++ = size; *p++ = size;
if (p + DIV_ROUNDUP(size, sizeof *p) > end) if (p + div_roundup(size, sizeof *p) > end)
goto overflow; goto overflow;
memcpy(p, closure->args[i].a->data, size); memcpy(p, closure->args[i].a->data, size);
p += DIV_ROUNDUP(size, sizeof *p); p += div_roundup(size, sizeof *p);
break; break;
default: default:
break; break;