fix not checking for string null byte
This commit is contained in:
parent
dd62b8570a
commit
e20e5ff3af
2 changed files with 21 additions and 2 deletions
15
server.c
15
server.c
|
@ -474,6 +474,16 @@ int jb_server_turn(struct jb_server *s)
|
||||||
static const char auth_data[] = "DATA\r\n";
|
static const char auth_data[] = "DATA\r\n";
|
||||||
static const int data_buffer_len = 4096;
|
static const int data_buffer_len = 4096;
|
||||||
|
|
||||||
|
// 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
|
||||||
|
// byte, we will reach the null bytes here at the end instead, thus preventing a
|
||||||
|
// crash and potential corruption. While this is a good "last line of defense"
|
||||||
|
// against such issues, it is much more important for these kinds of bugs to not
|
||||||
|
// exist in the first place. This mitigation will prevent, for example, ASAN from
|
||||||
|
// finding such bugs. It's recommended that you disable this padding outside
|
||||||
|
// of production so that you can find these bugs.
|
||||||
|
static const int data_buffer_padding = 0;
|
||||||
|
|
||||||
TRYST(poll(s->fds, s->fd_num, -1));
|
TRYST(poll(s->fds, s->fd_num, -1));
|
||||||
|
|
||||||
for (int i = 0; i < s->fd_num; i++) {
|
for (int i = 0; i < s->fd_num; i++) {
|
||||||
|
@ -511,9 +521,10 @@ int jb_server_turn(struct jb_server *s)
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
char data[data_buffer_len];
|
// We add padding. See above.
|
||||||
|
char data[data_buffer_len + data_buffer_padding];
|
||||||
memset(data, 0, sizeof(data));
|
memset(data, 0, sizeof(data));
|
||||||
ssize_t bytes = recv(fd, data, sizeof(data) - 1, 0);
|
ssize_t bytes = recv(fd, data, data_buffer_len, 0);
|
||||||
if (bytes <= 0) {
|
if (bytes <= 0) {
|
||||||
// error during recv() OR client disconnected, disconnect the client
|
// error during recv() OR client disconnected, disconnect the client
|
||||||
// TODO: should we actually do this?
|
// TODO: should we actually do this?
|
||||||
|
|
8
wire.c
8
wire.c
|
@ -62,7 +62,15 @@ char *wire_get_string_impl(wire_context_t *c, bool as_signature)
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// strings always end with a null byte
|
||||||
|
if (c->data[c->byte_cursor + len] != '\0') {
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
|
|
||||||
char *str = (char *)&c->data[c->byte_cursor];
|
char *str = (char *)&c->data[c->byte_cursor];
|
||||||
|
if (strlen(str) != len) {
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
|
|
||||||
c->byte_cursor = new_cursor;
|
c->byte_cursor = new_cursor;
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue