crypto-algorithms icon indicating copy to clipboard operation
crypto-algorithms copied to clipboard

Signed overflow undefined behavior in md5.c

Open skeeto opened this issue 2 years ago • 0 comments

The read from data is promoted to int, and so the left shift by 24 in md5.c may cause signed overflow. This requires cast to an unsigned int:

--- a/md5.c
+++ b/md5.c
@@ -42,3 +42,3 @@ void md5_transform(MD5_CTX *ctx, const BYTE data[])
        for (i = 0, j = 0; i < 16; ++i, j += 4)
-               m[i] = (data[j]) + (data[j + 1] << 8) + (data[j + 2] << 16) + (data[j + 3] << 24);
+               m[i] = (data[j]) + (data[j + 1] << 8) + (data[j + 2] << 16) + ((WORD)data[j + 3] << 24);

UBSan reveals this in the tests:

$ gcc -fsanitize=undefined,address md5_test.c md5.c
$ ./a.out 
md5.c:43:78: runtime error: left shift of 128 by 24 places cannot be represented in type 'int'

skeeto avatar Dec 12 '22 18:12 skeeto