undefined | Better HN

0 pointsloup-vaillant3y ago0 comments

This particular mistake is all the more infuriating because it comes from a precaution. Or trying to silence a compiler warning:

  partialBlock = (unsigned int)(dataByteLen - i);

Where both `dataByteLen` and `i` where actually `size_t`.

Assuming this is close enough to C, what happens is that we're converting a difference between `size_t` into a mere `unsigned`, and since they're not the same sizes on 64-bit platforms this can give `partialBlock` the wrong value, and the whole thing then snowballs into a catastrophic error that is not trivial to test because it only happens with huge buffer sizes.

The biggest mistake here is having written `(unsigned int)` instead of `(size_t)`. But the reason it happened in the first place is because they tried to do the right thing: writing the cast as a precaution, even though the following would have worked:

  partialBlock = dataByteLen - i;

I really can't fault them: because it was a difference it could theoretically yield a "negative" result, and therefore intuitively the type of a difference should be signed, so we should cast it back to unsigned to be crystal clear. I knew C was dangerous, but to be honest I didn't expect such a wicked mind game.

Now I'm going to have to take a look at my code.

0 comments

junon3y ago

> since they're not the same sizes on 64-bit platforms

Not guaranteed, but they certainly can be.

IYasha3y ago

umm... But how (uint - uint) can be negative? I thought it would warp around 0 into (UINT_MAX - leftovers).

loup-vaillantOP3y ago

You're right, it cannot, and does wrap around.

But some people might think that it could, so putting the cast could increase readability for them. A similar reasoning applies with operator precedence: even though it's very well defined, we tend to forget it, so instead we put additional parentheses. But in a couple specific cases it hurts more than it helps.

nayuki3y ago

So, (unsigned int) - (unsigned int) → (unsigned int) is guaranteed on every platform.

But relevant to the bad SHA-3 code, the resulting type of size_t - size_t can be surprising. First, size_t is not guaranteed to have any relation with unsigned int; it could be narrower, the same, or wider. For example, size_t could map to unsigned short, unsigned int, unsigned long, or unsigned long long.

If size_t is defined as unsigned short (note that size_t must be at least 16 bits), and short is 16 bits wide, and int is 32 bits wide, then the calculation of size_t - size_t will be promoted to (signed int) - (signed int), and thus the result will have the type signed int.

j / k navigate · click thread line to collapse