Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signed overflow undefined behavior in md5.c #33

Open
skeeto opened this issue Dec 12, 2022 · 0 comments · Fixed by nalgeon/sqlean#57
Open

Signed overflow undefined behavior in md5.c #33

skeeto opened this issue Dec 12, 2022 · 0 comments · Fixed by nalgeon/sqlean#57

Comments

@skeeto
Copy link

skeeto commented Dec 12, 2022

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'
taeasy added a commit to taeasy/crypto-algorithms that referenced this issue Jul 13, 2024
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:

B-Con#33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant