fix message bound issues
This commit is contained in:
parent
ce164d8d5d
commit
55b1a9b760
2 changed files with 46 additions and 9 deletions
54
server.c
54
server.c
|
@ -288,6 +288,11 @@ ssize_t bus_client_recv(bus_t *s, int i, void *buf, size_t n)
|
||||||
|
|
||||||
int bus_broadcast_message(bus_t *s, wire_message_t *msg, wire_context_t *ctx, char *sender_unique_name)
|
int bus_broadcast_message(bus_t *s, wire_message_t *msg, wire_context_t *ctx, char *sender_unique_name)
|
||||||
{
|
{
|
||||||
|
uint32_t body_end = ctx->byte_cursor + msg->body_length;
|
||||||
|
if (body_end >= ctx->data_len) {
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
|
||||||
int left = s->clients_count;
|
int left = s->clients_count;
|
||||||
for (int i = 0; i < BUS_MAX_CLIENTS && left > 0; i++) {
|
for (int i = 0; i < BUS_MAX_CLIENTS && left > 0; i++) {
|
||||||
if (s->clients[i].match_count <= 0) {
|
if (s->clients[i].match_count <= 0) {
|
||||||
|
@ -320,6 +325,10 @@ int bus_broadcast_message(bus_t *s, wire_message_t *msg, wire_context_t *ctx, ch
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// We rely on the byte_cursor being at the end of the message upon processing.
|
||||||
|
ctx->byte_cursor = body_end;
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -330,12 +339,12 @@ int bus_unicast_message(bus_t *s, wire_message_t *msg, wire_context_t *ctx, char
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
uint8_t reply_data[4096];
|
uint8_t reply_data[8192];
|
||||||
memset(reply_data, 0, 4096);
|
memset(reply_data, 0, 8192);
|
||||||
wire_context_t reply_ctx = {
|
wire_context_t reply_ctx = {
|
||||||
.byte_cursor = 0,
|
.byte_cursor = 0,
|
||||||
.data = reply_data,
|
.data = reply_data,
|
||||||
.data_len = 4096,
|
.data_len = 8192,
|
||||||
};
|
};
|
||||||
|
|
||||||
TRYST(wire_compose_unicast_reply(&reply_ctx, ctx, msg, sender_unique_name));
|
TRYST(wire_compose_unicast_reply(&reply_ctx, ctx, msg, sender_unique_name));
|
||||||
|
@ -511,6 +520,8 @@ int handle_remove_match(bus_t *s, int i, wire_message_t *msg, wire_context_t *ct
|
||||||
int handle_get_connection_stats(bus_t *s, int i, wire_message_t *msg, wire_context_t *ctx, wire_context_t *reply_ctx) {
|
int handle_get_connection_stats(bus_t *s, int i, wire_message_t *msg, wire_context_t *ctx, wire_context_t *reply_ctx) {
|
||||||
(void)ctx;
|
(void)ctx;
|
||||||
|
|
||||||
|
TRYPTR(wire_get_name_string(ctx));
|
||||||
|
|
||||||
// TODO: stub (always returns empty array)
|
// TODO: stub (always returns empty array)
|
||||||
printf("FIXME: STUB: GetConnectionStats\n");
|
printf("FIXME: STUB: GetConnectionStats\n");
|
||||||
|
|
||||||
|
@ -628,6 +639,11 @@ int bus_client_process_message(bus_t *s, int i, wire_context_t *ctx)
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
uint32_t body_end = ctx->byte_cursor + msg.body_length;
|
||||||
|
if (body_end >= ctx->data_len) {
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
|
||||||
wire_message_field_t *destination_field = &msg.fields[DBUS_HEADER_FIELD_DESTINATION];
|
wire_message_field_t *destination_field = &msg.fields[DBUS_HEADER_FIELD_DESTINATION];
|
||||||
wire_message_field_t *member_field = &msg.fields[DBUS_HEADER_FIELD_MEMBER];
|
wire_message_field_t *member_field = &msg.fields[DBUS_HEADER_FIELD_MEMBER];
|
||||||
|
|
||||||
|
@ -645,12 +661,12 @@ int bus_client_process_message(bus_t *s, int i, wire_context_t *ctx)
|
||||||
const bus_method_handler_t *handler = &bus_method_handlers[j];
|
const bus_method_handler_t *handler = &bus_method_handlers[j];
|
||||||
if (strcmp(handler->name, member_field->t.str) == 0) {
|
if (strcmp(handler->name, member_field->t.str) == 0) {
|
||||||
TRYST(handler->handler(s, i, &msg, ctx, &reply_ctx));
|
TRYST(handler->handler(s, i, &msg, ctx, &reply_ctx));
|
||||||
return 0;
|
goto end_nonfatal;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
printf("FIXME: daemon method '%s' is not implemented or invalid\n", member_field->t.str);
|
printf("FIXME: daemon method '%s' is not implemented or invalid\n", member_field->t.str);
|
||||||
_process_message_reply_error("org.freedesktop.DBus.Error.UnknownMethod");
|
_process_message_reply_error("org.freedesktop.DBus.Error.UnknownMethod");
|
||||||
return 0;
|
goto end_nonfatal;
|
||||||
}
|
}
|
||||||
|
|
||||||
// message needs to be routed
|
// message needs to be routed
|
||||||
|
@ -663,15 +679,32 @@ int bus_client_process_message(bus_t *s, int i, wire_context_t *ctx)
|
||||||
if (destination_field->present) {
|
if (destination_field->present) {
|
||||||
if (bus_unicast_message(s, &msg, ctx, destination_field->t.str, s->names[client->unique_name_index].name) < 0) {
|
if (bus_unicast_message(s, &msg, ctx, destination_field->t.str, s->names[client->unique_name_index].name) < 0) {
|
||||||
_process_message_reply_error("xyz.hippoz.jitterbug.UnicastFailed");
|
_process_message_reply_error("xyz.hippoz.jitterbug.UnicastFailed");
|
||||||
return 0;
|
goto end_nonfatal;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
if (bus_broadcast_message(s, &msg, ctx, s->names[client->unique_name_index].name) < 0) {
|
if (bus_broadcast_message(s, &msg, ctx, s->names[client->unique_name_index].name) < 0) {
|
||||||
_process_message_reply_error("xyz.hippoz.jitterbug.BroadcastFailed");
|
_process_message_reply_error("xyz.hippoz.jitterbug.BroadcastFailed");
|
||||||
return 0;
|
goto end_nonfatal;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
end_nonfatal:
|
||||||
|
if (ctx->byte_cursor != body_end) {
|
||||||
|
// The code above did not properly consume the entire message while parsing.
|
||||||
|
// This is bad, because we expect the byte_cursor after this function to be
|
||||||
|
// exactly at the end of a message. We rely on this invariant for message
|
||||||
|
// boundries. We can obviously just set the byte_cursor to the body_end
|
||||||
|
// that we have calculated and move on, however, not consuming the entire
|
||||||
|
// message is nonetheless a bug that should be fixed, or an indicator of
|
||||||
|
// another bug or something going wrong.
|
||||||
|
// In this case, I've decided to set the byte_cursor to the end of the
|
||||||
|
// body and print an error message. In the future, we may want to have
|
||||||
|
// separate debug builds, and maybe exit the program if we are in a
|
||||||
|
// debug build and we encounter something like this.
|
||||||
|
fprintf(stderr, "notice: parsing code did not consume the entire message (expected: %d, actual: %d)\n", body_end, ctx->byte_cursor);
|
||||||
|
ctx->byte_cursor = body_end;
|
||||||
|
}
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -685,7 +718,8 @@ int bus_client_drain_messages(bus_t *s, int ci, uint8_t *data, uint32_t data_len
|
||||||
|
|
||||||
TRYST(bus_client_process_message(s, ci, &ctx));
|
TRYST(bus_client_process_message(s, ci, &ctx));
|
||||||
|
|
||||||
for (int i = 0; i < 10; i++) {
|
int i = 0;
|
||||||
|
for (; i < 10; i++) {
|
||||||
if (ctx.byte_cursor >= ctx.data_len || ctx.data[ctx.byte_cursor] != 'l') {
|
if (ctx.byte_cursor >= ctx.data_len || ctx.data[ctx.byte_cursor] != 'l') {
|
||||||
// no more messages
|
// no more messages
|
||||||
break;
|
break;
|
||||||
|
@ -696,6 +730,8 @@ int bus_client_drain_messages(bus_t *s, int ci, uint8_t *data, uint32_t data_len
|
||||||
TRYST(bus_client_process_message(s, ci, &ctx));
|
TRYST(bus_client_process_message(s, ci, &ctx));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
printf("processed %d messages - cursor:%d; data_len:%ld\n", i + 1, ctx.byte_cursor, ctx.data_len);
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -706,7 +742,7 @@ int bus_turn(bus_t *s)
|
||||||
static const char auth_ok[] = "OK aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\r\n";
|
static const char auth_ok[] = "OK aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\r\n";
|
||||||
static const char auth_list[] = "REJECTED EXTERNAL\r\n";
|
static const char auth_list[] = "REJECTED EXTERNAL\r\n";
|
||||||
static const char auth_data[] = "DATA\r\n";
|
static const char auth_data[] = "DATA\r\n";
|
||||||
static const int data_buffer_len = 8192;
|
static const int data_buffer_len = 16384;
|
||||||
|
|
||||||
// We can keep a padding of null bytes for the data buffer. In the event that, for
|
// We can keep a padding of null bytes for the data buffer. In the event that, for
|
||||||
// example, a string function is called on a char array without a proper ending null
|
// example, a string function is called on a char array without a proper ending null
|
||||||
|
|
1
wire.c
1
wire.c
|
@ -383,6 +383,7 @@ int wire_compose_unicast_reply(wire_context_t *c, wire_context_t *msg_c, wire_me
|
||||||
|
|
||||||
memcpy(&c->data[c->byte_cursor], &msg_c->data[msg_c->byte_cursor], msg->body_length);
|
memcpy(&c->data[c->byte_cursor], &msg_c->data[msg_c->byte_cursor], msg->body_length);
|
||||||
c->byte_cursor += msg->body_length;
|
c->byte_cursor += msg->body_length;
|
||||||
|
msg_c->byte_cursor += msg->body_length;
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue