On Thu, Apr 17, 2014 at 11:20 AM, Ander Conselvan de Oliveira < [email protected]> wrote:
> From: Ander Conselvan de Oliveira <[email protected]> > > If a message was too big to fit in the connection buffer, the code > in wl_buffer_put would just write past the end of it. > > I haven't seen any real world use case that would trigger this bug, but > it was possible to trigger it by sending a long enough string to the > wl_data_source.offer request. > This will flat out kill the client, no? It seems like we should be able to handle that more gracefully; does the client have any idea what the buffer size is? > https://bugs.freedesktop.org/show_bug.cgi?id=69267 > --- > src/connection.c | 25 +++++++++++++++++-------- > tests/connection-test.c | 37 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+), 8 deletions(-) > > diff --git a/src/connection.c b/src/connection.c > index 40a2182..63b0592 100644 > --- a/src/connection.c > +++ b/src/connection.c > @@ -61,11 +61,18 @@ struct wl_connection { > int want_flush; > }; > > -static void > +static int > wl_buffer_put(struct wl_buffer *b, const void *data, size_t count) > { > uint32_t head, size; > > + if (count > sizeof(b->data)) { > + wl_log("Data too big for buffer (%d > %d).\n", > + count, sizeof(b->data)); > + errno = E2BIG; > + return -1; > + } > + > head = MASK(b->head); > if (head + count <= sizeof b->data) { > memcpy(b->data + head, data, count); > @@ -76,6 +83,8 @@ wl_buffer_put(struct wl_buffer *b, const void *data, > size_t count) > } > > b->head += count; > + > + return 0; > } > > static void > @@ -243,8 +252,8 @@ decode_cmsg(struct wl_buffer *buffer, struct msghdr > *msg) > size /= sizeof(int32_t); > for (i = 0; i < size; i++) > close(((int*)CMSG_DATA(cmsg))[i]); > - } else { > - wl_buffer_put(buffer, CMSG_DATA(cmsg), size); > + } else if (wl_buffer_put(buffer, CMSG_DATA(cmsg), size) < > 0) { > + return -1; > } > } > > @@ -350,7 +359,9 @@ wl_connection_write(struct wl_connection *connection, > return -1; > } > > - wl_buffer_put(&connection->out, data, count); > + if (wl_buffer_put(&connection->out, data, count) < 0) > + return -1; > + > connection->want_flush = 1; > > return 0; > @@ -367,7 +378,7 @@ wl_connection_queue(struct wl_connection *connection, > return -1; > } > > - wl_buffer_put(&connection->out, data, count); > + return wl_buffer_put(&connection->out, data, count); > > return 0; > You forgot to remove the "return 0;" here. > } > @@ -394,9 +405,7 @@ wl_connection_put_fd(struct wl_connection *connection, > int32_t fd) > return -1; > } > > - wl_buffer_put(&connection->fds_out, &fd, sizeof fd); > - > - return 0; > + return wl_buffer_put(&connection->fds_out, &fd, sizeof fd); > } > > const char * > diff --git a/tests/connection-test.c b/tests/connection-test.c > index 52d235d..1213875 100644 > --- a/tests/connection-test.c > +++ b/tests/connection-test.c > @@ -235,6 +235,27 @@ expected_fail_marshal(int expected_error, const char > *format, ...) > assert(errno == expected_error); > } > > +static void > +expected_fail_marshal_send(struct marshal_data *data, int expected_error, > + const char *format, ...) > +{ > + struct wl_closure *closure; > + static const uint32_t opcode = 4444; > + static struct wl_object sender = { NULL, NULL, 1234 }; > + struct wl_message message = { "test", format, NULL }; > + va_list ap; > + > + va_start(ap, format); > + closure = wl_closure_vmarshal(&sender, opcode, ap, &message); > + va_end(ap); > + > + assert(closure); > + assert(wl_closure_send(closure, data->write_connection) < 0); > + assert(errno == expected_error); > + > + wl_closure_destroy(closure); > +} > + > TEST(connection_marshal_nullables) > { > struct marshal_data data; > @@ -490,6 +511,22 @@ TEST(connection_marshal_alot) > release_marshal_data(&data); > } > > +TEST(connection_marshal_too_big) > +{ > + struct marshal_data data; > + char *big_string = malloc(5000); > + > + memset(big_string, ' ', 4999); > + big_string[4999] = '\0'; > + > + setup_marshal_data(&data); > + > + expected_fail_marshal_send(&data, E2BIG, "s", big_string); > + > + release_marshal_data(&data); > + free(big_string); > +} > + > static void > marshal_helper(const char *format, void *handler, ...) > { > -- > 1.8.3.2 > > _______________________________________________ > wayland-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > -- Jasper
_______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
