Demarshal robustness fixes

diff --git a/connection.c b/connection.c
index 76d4ce0..0ba5429 100644 (file)
--- a/connection.c
+++ b/connection.c
@@ -29,6 +29,7 @@
 #include <ffi.h>
 #include <assert.h>
 #include <fcntl.h>
+#include <unistd.h>
 
 #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 (file)
--- 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 (file)
--- 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;