On 13.11.20 15:03, Ilya Maximets wrote:
On 11/12/20 4:51 PM, Renat Nurgaliyev wrote:
Missing cast to 32 bits in overflow check makes this conditional never
be true. Because of this, computation of SHA-1 checksum will be wrong
for any data that is bigger than 512 megabytes, which in bits is the
boundary of 32 bits integer.
In practice it means that any big OVN southbound database, with
transactions bigger than 512 megabytes, is considered corrupt and
ovsdb-server will refuse to work with the database.
Signed-off-by: Renat Nurgaliyev <[email protected]>
---
lib/sha1.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/sha1.c b/lib/sha1.c
index 4f48ef210..4889c976b 100644
--- a/lib/sha1.c
+++ b/lib/sha1.c
@@ -202,7 +202,7 @@ sha1_update(struct sha1_ctx *ctx, const void *buffer_,
size_t count)
const uint8_t *buffer = buffer_;
unsigned int i;
- if ((ctx->count_lo + (count << 3)) < ctx->count_lo) {
+ if ((uint32_t)(ctx->count_lo + (count << 3)) < ctx->count_lo) {
ctx->count_hi++;
}
ctx->count_lo += count << 3;
Good catch! Thanks!
Looking at the code and comparing it with original Apache Portable Runtime
library it seems that the actual issue is that type of the 'count' changed
from 'unsigned int' in the original code to 'size_t' that is 64bit on modern
systems. So, I think, instead of the cast we need to change the type
of 'count' here and in 'sha1_bytes()' to uint32_t, so we will cover this
case and possible other issues related to this. One more thing why we need
to change the type is that I'm not sure if functions will work correctly in
all cases if someone will call them with chunks of data > UINT32_MAX in size,
so it's better to protect ourselves by making it clear that chunk size should
fit into uint32_t.
What do you think?
Best regards, Ilya Maximets.
Hi,
yes, I think it makes sense. I will check couple of things and will submit a
new patch in next days.
Thanks,
Renat.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
