From: Kristian Høgsberg Date: Mon, 9 Aug 2010 18:34:11 +0000 (-0400) Subject: Demarshal robustness fixes X-Git-Tag: 0.85.0~491 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5b0079aed5555ef82f86c95516e90a9a8f018324;p=profile%2Fivi%2Fwayland.git Demarshal robustness fixes --- diff --git a/connection.c b/connection.c index 76d4ce0..0ba5429 100644 --- a/connection.c +++ b/connection.c @@ -29,6 +29,7 @@ #include #include #include +#include #include "wayland-util.h" #include "connection.h" @@ -279,7 +280,7 @@ wl_connection_vmarshal(struct wl_connection *connection, wl_connection_write(connection, args, size); } -void +int wl_connection_demarshal(struct wl_connection *connection, uint32_t size, struct wl_hash_table *objects, @@ -289,8 +290,8 @@ wl_connection_demarshal(struct wl_connection *connection, { ffi_type *types[20]; ffi_cif cif; - uint32_t *p, result, length; - int i, count; + uint32_t *p, *next, *end, result, length; + int i, count, ret = 0; union { uint32_t uint32; char *string; @@ -305,12 +306,12 @@ wl_connection_demarshal(struct wl_connection *connection, count = strlen(message->signature) + 2; if (count > ARRAY_LENGTH(types)) { printf("too many args (%d)\n", count); - return; + assert(0); } if (sizeof buffer < size) { printf("request too big, should malloc tmp buffer here\n"); - return; + assert(0); } types[0] = &ffi_type_pointer; @@ -323,7 +324,17 @@ wl_connection_demarshal(struct wl_connection *connection, wl_connection_copy(connection, buffer, size); p = &buffer[2]; + end = (uint32_t *) ((char *) (p + size)); for (i = 2; i < count; i++) { + if (p + 1 > end) { + printf("message too short, " + "object (%d), message %s(%s)\n", + *p, message->name, message->signature); + errno = EINVAL; + ret = -1; + goto out; + } + switch (message->signature[i - 2]) { case 'u': case 'i': @@ -333,21 +344,37 @@ wl_connection_demarshal(struct wl_connection *connection, case 's': types[i] = &ffi_type_pointer; length = *p++; + + next = p + DIV_ROUNDUP(length, sizeof *p); + if (next > end) { + printf("message too short, " + "object (%d), message %s(%s)\n", + *p, message->name, message->signature); + errno = EINVAL; + ret = -1; + goto out; + } + values[i].string = malloc(length + 1); if (values[i].string == NULL) { - /* FIXME: Send NO_MEMORY */ - return; + errno = ENOMEM; + ret = -1; + goto out; } memcpy(values[i].string, p, length); values[i].string[length] = '\0'; - p += DIV_ROUNDUP(length, sizeof *p); + p = next; break; case 'o': types[i] = &ffi_type_pointer; object = wl_hash_table_lookup(objects, *p); - if (object == NULL && *p != 0) + if (object == NULL && *p != 0) { printf("unknown object (%d), message %s(%s)\n", *p, message->name, message->signature); + errno = EINVAL; + ret = -1; + goto out; + } values[i].object = object; p++; break; @@ -355,26 +382,46 @@ wl_connection_demarshal(struct wl_connection *connection, types[i] = &ffi_type_uint32; values[i].new_id = *p; object = wl_hash_table_lookup(objects, *p); - if (object != NULL) - printf("object already exists (%d)\n", *p); + if (object != NULL) { + printf("not a new object (%d), " + "message %s(%s)\n", + *p, message->name, message->signature); + errno = EINVAL; + ret = -1; + goto out; + } p++; break; case 'a': types[i] = &ffi_type_pointer; length = *p++; - values[i].array = malloc(length + sizeof *values[i].array); + + next = p + DIV_ROUNDUP(length, sizeof *p); + if (next > end) { + printf("message too short, " + "object (%d), message %s(%s)\n", + *p, message->name, message->signature); + errno = EINVAL; + ret = -1; + goto out; + } + + values[i].array = + malloc(length + sizeof *values[i].array); if (values[i].array == NULL) { - /* FIXME: Send NO_MEMORY */ - return; + errno = ENOMEM; + ret = -1; + goto out; } values[i].array->size = length; values[i].array->alloc = 0; values[i].array->data = values[i].array + 1; memcpy(values[i].array->data, p, length); - p += DIV_ROUNDUP(length, sizeof *p); + p = next; break; default: printf("unknown type\n"); + assert(0); break; } args[i] = &values[i]; @@ -383,6 +430,8 @@ wl_connection_demarshal(struct wl_connection *connection, ffi_prep_cif(&cif, FFI_DEFAULT_ABI, count, &ffi_type_uint32, types); ffi_call(&cif, func, &result, args); + out: + count = i; for (i = 2; i < count; i++) { switch (message->signature[i - 2]) { case 's': @@ -393,4 +442,6 @@ wl_connection_demarshal(struct wl_connection *connection, break; } } + + return ret; } diff --git a/connection.h b/connection.h index 30e82e1..7f65e87 100644 --- a/connection.h +++ b/connection.h @@ -48,11 +48,11 @@ void wl_connection_vmarshal(struct wl_connection *connection, uint32_t opcode, va_list ap, const struct wl_message *message); -void wl_connection_demarshal(struct wl_connection *connection, - uint32_t size, - struct wl_hash_table *objects, - void (*func)(void), - void *data, struct wl_object *target, - const struct wl_message *message); +int wl_connection_demarshal(struct wl_connection *connection, + uint32_t size, + struct wl_hash_table *objects, + void (*func)(void), + void *data, struct wl_object *target, + const struct wl_message *message); #endif diff --git a/wayland.c b/wayland.c index 8802c33..ee7150c 100644 --- a/wayland.c +++ b/wayland.c @@ -86,7 +86,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data) struct wl_object *object; uint32_t p[2], opcode, size; uint32_t cmask = 0; - int len; + int len, ret; if (mask & WL_EVENT_READABLE) cmask |= WL_CONNECTION_READABLE; @@ -114,7 +114,7 @@ wl_client_connection_data(int fd, uint32_t mask, void *data) len -= size; continue; } - + if (opcode >= object->interface->method_count) { wl_client_post_event(client, &client->display->base, WL_DISPLAY_INVALID_METHOD, p[0], opcode); @@ -122,14 +122,22 @@ wl_client_connection_data(int fd, uint32_t mask, void *data) len -= size; continue; } - - wl_connection_demarshal(client->connection, - size, - client->display->objects, - object->implementation[opcode], - client, - object, - &object->interface->methods[opcode]); + + ret = wl_connection_demarshal(client->connection, + size, + client->display->objects, + object->implementation[opcode], + client, + object, + &object->interface->methods[opcode]); + + if (ret < 0 && errno == EINVAL) + wl_client_post_event(client, &client->display->base, + WL_DISPLAY_INVALID_METHOD, + p[0], opcode); + if (ret < 0 && errno == ENOMEM) + wl_client_post_event(client, &client->display->base, + WL_DISPLAY_NO_MEMORY); wl_connection_consume(connection, size); len -= size;