From 07b5f3e315c179f7de41c3b519aaacb1563c2f87 Mon Sep 17 00:00:00 2001 From: Atle Solbakken Date: Fri, 22 Mar 2024 07:49:25 +0100 Subject: [PATCH 1/3] [atlesn] Fix misc. usage of uninitialized memory --- src/convert.c | 2 +- src/uv_encoding.c | 2 +- src/uv_tcp_connect.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/convert.c b/src/convert.c index e136bfa6..90cc4dea 100644 --- a/src/convert.c +++ b/src/convert.c @@ -221,7 +221,7 @@ int convertToLeader(struct raft *r) r->barrier.type = RAFT_BARRIER; r->barrier.term = r->current_term; r->barrier.buf.len = 8; - r->barrier.buf.base = raft_malloc(r->barrier.buf.len); + r->barrier.buf.base = raft_calloc(1, r->barrier.buf.len); if (r->barrier.buf.base == NULL) { rv = RAFT_NOMEM; diff --git a/src/uv_encoding.c b/src/uv_encoding.c index 747cb2a9..358fa3aa 100644 --- a/src/uv_encoding.c +++ b/src/uv_encoding.c @@ -237,7 +237,7 @@ int uvEncodeMessage(const struct raft_message *message, return RAFT_MALFORMED; }; - header.base = raft_malloc(header.len); + header.base = raft_calloc(1, header.len); if (header.base == NULL) { goto oom; } diff --git a/src/uv_tcp_connect.c b/src/uv_tcp_connect.c index 1d00e111..738843ef 100644 --- a/src/uv_tcp_connect.c +++ b/src/uv_tcp_connect.c @@ -57,7 +57,7 @@ static int uvTcpEncodeHandshake(raft_id id, const char *address, uv_buf_t *buf) sizeof(uint64_t) + /* Server ID. */ sizeof(uint64_t) /* Size of the address buffer */; buf->len += address_len; - buf->base = RaftHeapMalloc(buf->len); + buf->base = RaftHeapCalloc(1, buf->len); if (buf->base == NULL) { return RAFT_NOMEM; } From 515b97b02f8dee6a37d87904ed24e239cf23583f Mon Sep 17 00:00:00 2001 From: Atle Solbakken Date: Sat, 23 Mar 2024 07:16:08 +0100 Subject: [PATCH 2/3] [atlesn] Replace callocs() with explicit initialization of unused fields or areas --- src/convert.c | 4 +++- src/uv_encoding.c | 7 +++++-- src/uv_tcp_connect.c | 15 +++++++++------ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/convert.c b/src/convert.c index 90cc4dea..d133736e 100644 --- a/src/convert.c +++ b/src/convert.c @@ -221,13 +221,15 @@ int convertToLeader(struct raft *r) r->barrier.type = RAFT_BARRIER; r->barrier.term = r->current_term; r->barrier.buf.len = 8; - r->barrier.buf.base = raft_calloc(1, r->barrier.buf.len); + r->barrier.buf.base = raft_malloc(r->barrier.buf.len); if (r->barrier.buf.base == NULL) { rv = RAFT_NOMEM; goto err; } + *(uint64_t *) r->barrier.buf.base = 0; + r->barrier.batch = r->barrier.buf.base; rv = ClientSubmit(r, &r->barrier, 1); diff --git a/src/uv_encoding.c b/src/uv_encoding.c index 358fa3aa..bc1d31e9 100644 --- a/src/uv_encoding.c +++ b/src/uv_encoding.c @@ -237,7 +237,7 @@ int uvEncodeMessage(const struct raft_message *message, return RAFT_MALFORMED; }; - header.base = raft_calloc(1, header.len); + header.base = raft_malloc(header.len); if (header.base == NULL) { goto oom; } @@ -337,7 +337,10 @@ void uvEncodeBatchHeader(const struct raft_entry *entries, /* Message type (Either RAFT_COMMAND or RAFT_CHANGE) */ bytePut8(&cursor, (uint8_t)entry->type); - cursor = (uint8_t *)cursor + 3; /* Unused */ + /* Unused */ + bytePut8(&cursor, 0); + bytePut8(&cursor, 0); + bytePut8(&cursor, 0); /* Size of the log entry data, little endian. */ bytePut32(&cursor, (uint32_t)entry->buf.len); diff --git a/src/uv_tcp_connect.c b/src/uv_tcp_connect.c index 738843ef..5fa4456f 100644 --- a/src/uv_tcp_connect.c +++ b/src/uv_tcp_connect.c @@ -52,20 +52,23 @@ struct uvTcpConnect static int uvTcpEncodeHandshake(raft_id id, const char *address, uv_buf_t *buf) { uint8_t *cursor; - size_t address_len = bytePad64(strlen(address) + 1); + size_t address_len = strlen(address) + 1; + size_t address_len_padded = bytePad64(address_len); buf->len = sizeof(uint64_t) + /* Protocol version. */ sizeof(uint64_t) + /* Server ID. */ - sizeof(uint64_t) /* Size of the address buffer */; - buf->len += address_len; - buf->base = RaftHeapCalloc(1, buf->len); + sizeof(uint64_t) + /* Size of the address buffer */ + address_len_padded; + buf->base = RaftHeapMalloc(buf->len); if (buf->base == NULL) { return RAFT_NOMEM; } cursor = (uint8_t *)buf->base; bytePut64(&cursor, UV__TCP_HANDSHAKE_PROTOCOL); bytePut64(&cursor, id); - bytePut64(&cursor, address_len); - strcpy((char *)cursor, address); + bytePut64(&cursor, address_len_padded); + memcpy(cursor, address, address_len); + cursor += address_len; + memset(cursor, 0, address_len_padded - address_len); return 0; } From e99dde7882d0f77406da160e133bee983163dde8 Mon Sep 17 00:00:00 2001 From: Atle Solbakken Date: Sat, 23 Mar 2024 08:35:23 +0100 Subject: [PATCH 3/3] [atlesn] Fix usage of uninitialized memory during snapshoting --- src/configuration.c | 11 +++++++++-- src/configuration.h | 6 ++++-- src/uv_encoding.c | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/configuration.c b/src/configuration.c index 1c8415e8..73bd3959 100644 --- a/src/configuration.c +++ b/src/configuration.c @@ -282,9 +282,12 @@ size_t configurationEncodedSize(const struct raft_configuration *c) return bytePad64(n); } -void configurationEncodeToBuf(const struct raft_configuration *c, void *buf) +void configurationEncodeToBuf(const struct raft_configuration *c, + void *buf, + size_t buf_len) { uint8_t *cursor = buf; + uint8_t *end = cursor + buf_len; unsigned i; /* Encoding format version */ @@ -301,6 +304,10 @@ void configurationEncodeToBuf(const struct raft_configuration *c, void *buf) assert(server->role < 255); bytePut8(&cursor, (uint8_t)server->role); }; + + assert(cursor <= end); + + memset(cursor, 0, (size_t) (end - cursor)); } int configurationEncode(const struct raft_configuration *c, @@ -321,7 +328,7 @@ int configurationEncode(const struct raft_configuration *c, goto err; } - configurationEncodeToBuf(c, buf->base); + configurationEncodeToBuf(c, buf->base, buf->len); return 0; diff --git a/src/configuration.h b/src/configuration.h index 87111108..26b5970c 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -96,8 +96,10 @@ int configurationCopy(const struct raft_configuration *src, size_t configurationEncodedSize(const struct raft_configuration *c); /* Encode the given configuration object to the given pre-allocated buffer, - * which is assumed to be at least configurationEncodedSize(c) bytes. */ -void configurationEncodeToBuf(const struct raft_configuration *c, void *buf); + * which must be at least configurationEncodedSize(c) bytes. */ +void configurationEncodeToBuf(const struct raft_configuration *c, + void *buf, + size_t buf_len); /* Encode the given configuration object. The memory of the returned buffer is * allocated using raft_malloc(), and client code is responsible for releasing diff --git a/src/uv_encoding.c b/src/uv_encoding.c index bc1d31e9..4be0bc3c 100644 --- a/src/uv_encoding.c +++ b/src/uv_encoding.c @@ -180,7 +180,7 @@ static void encodeInstallSnapshot(const struct raft_install_snapshot *p, bytePut64(&cursor, p->conf_index); /* Configuration's index */ bytePut64(&cursor, conf_size); /* Length of configuration */ - configurationEncodeToBuf(&p->conf, cursor); /* Configuration data */ + configurationEncodeToBuf(&p->conf, cursor, conf_size); /* Configuration data */ cursor = (uint8_t *)cursor + conf_size; bytePut64(&cursor, p->data.len); /* Length of snapshot data */