Closed guidovranken closed 3 years ago
Thank you for this report. This is not a security issue, which is probably why the people at secure@microsoft.commailto:secure@microsoft.com didn’t know what to do.
Looking at the specs of the C language, I see that the byte value is promoted to an int, and the left shift by 24 of a byte value >= 128 makes the value larger than can be represented in a signed int, it would need an unsigned int. In fact, C only guarantees that int is 16 bits long, so this code is even more flawed.
The right fix will be to insert casts to UINT32; the shift operations are well-defined on unsigned 32-bit values.
I don’t think this is urgent, so I’ll fix it the next time I’m changing things.
Thanks again,
Niels
I believe this is resolved in the latest push.
I reported this to secure@microsoft.com (per your explicit request to report potential security bugs there, I personally don't care), but they don't understand what I'm talking about.
The problem is that you don't perform the necessary casting: https://github.com/microsoft/SymCrypt/blob/263e3e6550a7ca5386fa5edcdb7dbd52702c0d50/inc/symcrypt_internal.h#L524