From 9ee2d56e806b8018fa3ae354a65f1e70bf73dede Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Fri, 23 Sep 2022 18:39:20 +0200 Subject: keccak: Use size_t to avoid integer overflow * cipher/keccak-armv7-neon.S: Fix function name in comment and change parameter type to size_t. * cipher/keccak.c (keccak_ops_t): Change absorb function signature to use size_t. (keccak_absorb_lanes64_avx512): Change nlanes type to size_t. (_gcry_keccak_absorb_lanes64_armv7_neon): Ditto. (keccak_absorb_lanes64_armv7_neon): Ditto. (keccak_absorb_lanes32bi): Ditto. (keccak_absorb_lanes32bi_bmi2): Ditto. (keccak_write): Change nlanes variable to use size_t and avoid overflow when calculating count. * cipher/keccak_permute_64.h (KECCAK_F1600_ABSORB_FUNC_NAME): Change nlanes argument to use size_t. -- Cherry-pick master commit of: 9c828129b2058c3f36e07634637929a54e8377ee Any input to the SHA3 functions > 4GB was giving wrong result when it was invoked in one-shot, while working correctly when it was fed by chunks. It turned out that the calculation in the `keccak_write` overflows the `unsigned int` type (`nlanes * 8` does not fit 32b when the `inlen` > 4GB). GnuPG-bug-id: 6217 Signed-off-by: Jakub Jelen --- cipher/keccak-armv7-neon.S | 10 +++++----- cipher/keccak.c | 16 ++++++++-------- cipher/keccak_permute_64.h | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cipher/keccak-armv7-neon.S b/cipher/keccak-armv7-neon.S index 0bec8d50..28a284a1 100644 --- a/cipher/keccak-armv7-neon.S +++ b/cipher/keccak-armv7-neon.S @@ -467,11 +467,11 @@ _gcry_keccak_permute_armv7_neon: .ltorg .size _gcry_keccak_permute_armv7_neon,.-_gcry_keccak_permute_armv7_neon; -@//unsigned _gcry_keccak_permute_armv7_neon(u64 *state, @r4 -@ int pos, @r1 -@ const byte *lanes, @r2 -@ unsigned int nlanes, @r3 -@ int blocklanes) @ r5 callable from C +@//unsigned _gcry_keccak_absorb_lanes64_armv7_neon(u64 *state, @r4 +@ int pos, @r1 +@ const byte *lanes, @r2 +@ size_t nlanes, @r3 +@ int blocklanes) @ r5 callable from C .p2align 3 .global _gcry_keccak_absorb_lanes64_armv7_neon .type _gcry_keccak_absorb_lanes64_armv7_neon,%function; diff --git a/cipher/keccak.c b/cipher/keccak.c index f3502022..11e64b3e 100644 --- a/cipher/keccak.c +++ b/cipher/keccak.c @@ -111,7 +111,7 @@ typedef struct { unsigned int (*permute)(KECCAK_STATE *hd); unsigned int (*absorb)(KECCAK_STATE *hd, int pos, const byte *lanes, - unsigned int nlanes, int blocklanes); + size_t nlanes, int blocklanes); unsigned int (*extract) (KECCAK_STATE *hd, unsigned int pos, byte *outbuf, unsigned int outlen); } keccak_ops_t; @@ -434,7 +434,7 @@ static const keccak_ops_t keccak_bmi2_64_ops = unsigned int _gcry_keccak_permute_armv7_neon(u64 *state); unsigned int _gcry_keccak_absorb_lanes64_armv7_neon(u64 *state, int pos, const byte *lanes, - unsigned int nlanes, + size_t nlanes, int blocklanes); static unsigned int keccak_permute64_armv7_neon(KECCAK_STATE *hd) @@ -444,7 +444,7 @@ static unsigned int keccak_permute64_armv7_neon(KECCAK_STATE *hd) static unsigned int keccak_absorb_lanes64_armv7_neon(KECCAK_STATE *hd, int pos, const byte *lanes, - unsigned int nlanes, int blocklanes) + size_t nlanes, int blocklanes) { if (blocklanes < 0) { @@ -492,7 +492,7 @@ static const keccak_ops_t keccak_armv7_neon_64_ops = static unsigned int keccak_absorb_lanes32bi(KECCAK_STATE *hd, int pos, const byte *lanes, - unsigned int nlanes, int blocklanes) + size_t nlanes, int blocklanes) { unsigned int burn = 0; @@ -574,7 +574,7 @@ keccak_absorb_lane32bi_bmi2(u32 *lane, u32 x0, u32 x1) static unsigned int keccak_absorb_lanes32bi_bmi2(KECCAK_STATE *hd, int pos, const byte *lanes, - unsigned int nlanes, int blocklanes) + size_t nlanes, int blocklanes) { unsigned int burn = 0; @@ -794,7 +794,8 @@ keccak_write (void *context, const void *inbuf_arg, size_t inlen) const byte *inbuf = inbuf_arg; unsigned int nburn, burn = 0; unsigned int count, i; - unsigned int pos, nlanes; + unsigned int pos; + size_t nlanes; #ifdef USE_S390X_CRYPTO if (ctx->kimd_func) @@ -839,8 +840,7 @@ keccak_write (void *context, const void *inbuf_arg, size_t inlen) burn = nburn > burn ? nburn : burn; inlen -= nlanes * 8; inbuf += nlanes * 8; - count += nlanes * 8; - count = count % bsize; + count = ((size_t) count + nlanes * 8) % bsize; } if (inlen) diff --git a/cipher/keccak_permute_64.h b/cipher/keccak_permute_64.h index b28c871e..45ef462f 100644 --- a/cipher/keccak_permute_64.h +++ b/cipher/keccak_permute_64.h @@ -292,7 +292,7 @@ KECCAK_F1600_PERMUTE_FUNC_NAME(KECCAK_STATE *hd) static unsigned int KECCAK_F1600_ABSORB_FUNC_NAME(KECCAK_STATE *hd, int pos, const byte *lanes, - unsigned int nlanes, int blocklanes) + size_t nlanes, int blocklanes) { unsigned int burn = 0; -- cgit v1.2.1