What the developer wrote
Authored by Samy
crypto: handle unaligned Groestl input
What changed, and why it matters
This commit fixes a crash or incorrect-behavior bug in Monero's Groestl hash function when given data that is not aligned to a 4-byte memory boundary. The old code read the input directly as 32-bit words, which can fail on some processors or compilers that require aligned memory access. The fix copies the bytes one word at a time using memcpy, which is safe for any memory address. A new unit test verifies that hashing the same bytes at different alignments produces the same result.
Treat as a hardening/bug-fix commit. No immediate emergency response is indicated, but the fix should be included in releases because unaligned Groestl inputs could cause crashes or consensus-relevant hash mismatches on strict-alignment platforms. Review whether other hash functions in the same directory cast byte buffers to wider types without alignment checks.
Security signals we found
Alignment-sensitive memory read replaced with byte-safe memcpy
New unit test explicitly targets unaligned input behavior
Change is in a cryptographic hash implementation (Groestl)
Evidence from the diff
The Groestl-512 compression function F512 previously accepted a const uint32_t message pointer and dereferenced it word-by-word. When called from Transform with an arbitrary uint8_t input, unaligned input buffers could trigger undefined behavior or alignment faults on strict-alignment architectures. The patch changes F512 to take const uint8_t* and loads each word with memcpy, avoiding alignment assumptions. A unit test exercises offsets 0-3 within an over-allocated buffer to confirm identical hashes.
Changed components
src/crypto/groestl.ctests/unit_tests/crypto.cppInspect captured patch +28 / −4
diff --git a/src/crypto/groestl.c b/src/crypto/groestl.c
index d5e2989..6f3cfb3 100644
--- a/src/crypto/groestl.c
+++ b/src/crypto/groestl.c
@@ -9,6 +9,7 @@
*/
#include <stddef.h>
+#include <string.h>
#include "groestl.h"
#include "groestl_tables.h"
@@ -124,7 +125,7 @@ static void RND512Q(uint8_t *x, uint32_t *y, uint32_t r) {
}
/* compute compression function (short variants) */
-static void F512(uint32_t *h, const uint32_t *m) {
+static void F512(uint32_t *h, const uint8_t *m) {
int i;
uint32_t Ptmp[2*COLS512];
uint32_t Qtmp[2*COLS512];
@@ -132,8 +133,10 @@ static void F512(uint32_t *h, const uint32_t *m) {
uint32_t z[2*COLS512];
for (i = 0; i < 2*COLS512; i++) {
- z[i] = m[i];
- Ptmp[i] = h[i]^m[i];
+ uint32_t word;
+ memcpy(&word, m + i*sizeof(word), sizeof(word));
+ z[i] = word;
+ Ptmp[i] = h[i]^word;
}
/* compute Q(m) */
@@ -175,7 +178,7 @@ static void Transform(hashState *ctx,
/* digest message, one block at a time */
for (; msglen >= SIZE512;
msglen -= SIZE512, input += SIZE512) {
- F512(ctx->chaining,(uint32_t*)input);
+ F512(ctx->chaining, input);
/* increment block counter */
ctx->block_counter1++;
diff --git a/tests/unit_tests/crypto.cpp b/tests/unit_tests/crypto.cpp
index e0e4713..70a2465 100644
--- a/tests/unit_tests/crypto.cpp
+++ b/tests/unit_tests/crypto.cpp
@@ -27,6 +27,7 @@
// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <cstdint>
+#include <cstring>
#include <gtest/gtest.h>
#include <memory>
#include <sstream>
@@ -102,6 +103,26 @@ TEST(Crypto, null_keys)
ASSERT_EQ(memcmp(crypto::null_pkey.data, zero, 32), 0);
}
+TEST(Crypto, groestl_unaligned_input)
+{
+ constexpr std::size_t input_size = 128;
+ alignas(std::uint32_t) std::uint8_t aligned[input_size];
+ alignas(std::uint32_t) std::uint8_t input[input_size + alignof(std::uint32_t)];
+ char expected[crypto::HASH_SIZE];
+ char actual[crypto::HASH_SIZE];
+
+ for (std::size_t i = 0; i < input_size; ++i)
+ aligned[i] = static_cast<std::uint8_t>(i);
+ crypto::hash_extra_groestl(aligned, input_size, expected);
+
+ for (std::size_t offset = 0; offset < alignof(std::uint32_t); ++offset)
+ {
+ std::memcpy(input + offset, aligned, input_size);
+ crypto::hash_extra_groestl(input + offset, input_size, actual);
+ EXPECT_EQ(0, std::memcmp(expected, actual, sizeof(expected)));
+ }
+}
+
TEST(Crypto, verify_32)
{
// all bytes are treated the same, so we can brute force just one byte
Why this scored 44/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.