From 7b5b910f3f065d6486b12e18faf4ebeedb4f796a Mon Sep 17 00:00:00 2001 From: SChernykh Date: Fri, 8 Apr 2022 22:34:37 +0200 Subject: [PATCH] Undefined behaviour fixes --- src/keccak.cpp | 2 +- src/p2p_server.cpp | 31 ++++++++++++++++++++----------- src/util.h | 10 ++++++++++ 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/keccak.cpp b/src/keccak.cpp index 349036f..c44a45b 100644 --- a/src/keccak.cpp +++ b/src/keccak.cpp @@ -159,7 +159,7 @@ NOINLINE void keccak(const uint8_t* in, int inlen, uint8_t* md, int mdlen) for (; inlen >= rsiz; inlen -= rsiz, in += rsiz) { for (int i = 0; i < rsizw; i++) { - st[i] ^= ((uint64_t*)in)[i]; + st[i] ^= read_unaligned(reinterpret_cast(in) + i); } keccakf(st); } diff --git a/src/p2p_server.cpp b/src/p2p_server.cpp index 3ba7c56..d15ffb3 100644 --- a/src/p2p_server.cpp +++ b/src/p2p_server.cpp @@ -800,21 +800,27 @@ void P2PServer::on_broadcast() LOGINFO(6, "sending BLOCK_BROADCAST (pruned) to " << log::Gray() << static_cast(client->m_addrString)); *(p++) = static_cast(MessageId::BLOCK_BROADCAST); - *reinterpret_cast(p) = static_cast(data->pruned_blob.size()); + const uint32_t len = static_cast(data->pruned_blob.size()); + memcpy(p, &len, sizeof(uint32_t)); p += sizeof(uint32_t); - memcpy(p, data->pruned_blob.data(), data->pruned_blob.size()); - p += data->pruned_blob.size(); + if (len) { + memcpy(p, data->pruned_blob.data(), len); + p += len; + } } else { LOGINFO(5, "sending BLOCK_BROADCAST (full) to " << log::Gray() << static_cast(client->m_addrString)); *(p++) = static_cast(MessageId::BLOCK_BROADCAST); - *reinterpret_cast(p) = static_cast(data->blob.size()); + const uint32_t len = static_cast(data->blob.size()); + memcpy(p, &len, sizeof(uint32_t)); p += sizeof(uint32_t); - memcpy(p, data->blob.data(), data->blob.size()); - p += data->blob.size(); + if (len) { + memcpy(p, data->blob.data(), len); + p += len; + } } return p - p0; @@ -1219,7 +1225,7 @@ bool P2PServer::P2PClient::on_read(char* data, uint32_t size) LOGINFO(5, "peer " << log::Gray() << static_cast(m_addrString) << log::NoColor() << " sent BLOCK_RESPONSE"); if (bytes_left >= 1 + sizeof(uint32_t)) { - const uint32_t block_size = *reinterpret_cast(buf + 1); + const uint32_t block_size = read_unaligned(reinterpret_cast(buf + 1)); if (bytes_left >= 1 + sizeof(uint32_t) + block_size) { bytes_read = 1 + sizeof(uint32_t) + block_size; @@ -1237,7 +1243,7 @@ bool P2PServer::P2PClient::on_read(char* data, uint32_t size) LOGINFO(6, "peer " << log::Gray() << static_cast(m_addrString) << log::NoColor() << " sent BLOCK_BROADCAST"); if (bytes_left >= 1 + sizeof(uint32_t)) { - const uint32_t block_size = *reinterpret_cast(buf + 1); + const uint32_t block_size = read_unaligned(reinterpret_cast(buf + 1)); if (bytes_left >= 1 + sizeof(uint32_t) + block_size) { bytes_read = 1 + sizeof(uint32_t) + block_size; if (!on_block_broadcast(buf + 1 + sizeof(uint32_t), block_size)) { @@ -1678,11 +1684,14 @@ bool P2PServer::P2PClient::on_block_request(const uint8_t* buf) LOGINFO(5, "sending BLOCK_RESPONSE"); *(p++) = static_cast(MessageId::BLOCK_RESPONSE); - *reinterpret_cast(p) = static_cast(blob.size()); + const uint32_t len = static_cast(blob.size()); + memcpy(p, &len, sizeof(uint32_t)); p += sizeof(uint32_t); - memcpy(p, blob.data(), blob.size()); - p += blob.size(); + if (len) { + memcpy(p, blob.data(), len); + p += len; + } return p - p0; }); diff --git a/src/util.h b/src/util.h index 073f334..453496f 100644 --- a/src/util.h +++ b/src/util.h @@ -133,6 +133,16 @@ const uint8_t* readVarint(const uint8_t* data, const uint8_t* data_end, T& b) return nullptr; } +template +FORCEINLINE T read_unaligned(const T* p) +{ + static_assert(std::is_integral::value, "T must be an integer type"); + + T result; + memcpy(&result, p, sizeof(T)); + return result; +} + template FORCEINLINE constexpr size_t array_size(T(&)[N]) { return N; } template FORCEINLINE constexpr size_t array_size(T(U::*)[N]) { return N; }