Strengthen undefined behavior prevention in checkSignedBitfieldOverflow

[Fusl]

This one was found by afl++. Executing bitfield 0 set i64 0 1 triggers UBSan at the int64_t minincr = min - value; calculation. To fix the undefined behavior in the minincr calculation and strengthen the protection in the maxincr calculation, we cast both, the minuend and the subtrahend, to an unsigned int, do the calculation, and then cast the result back into a signed int.

[zuiderkwast]

Would it make sense to add bitfield 0 set i64 0 1 to some test case just to verify the fix?

[Fusl]

Would it make sense to add bitfield 0 set i64 0 1 to some test case just to verify the fix?

I tried adding a test case for this but couldn't trigger it outside of a binary built with afl-clang, maybe I was doing something wrong and didn't supply correct compilation flags when building a custom non-afl binary?

[zuiderkwast]

The cast to signed can be implicit, for brevity?

Suggested change

	int64_t maxincr = (int64_t)((uint64_t)max - (uint64_t)value);
	int64_t minincr = (int64_t)((uint64_t)min - (uint64_t)value);
	int64_t maxincr = (uint64_t)max - (uint64_t)value;
	int64_t minincr = (uint64_t)min - (uint64_t)value;

[Fusl]

I made this explicit since clang-tidy otherwise complains about it:

[zuiderkwast]

Oh, is implicit cast different to explicit? I didn't know...

Let's keep it explicit then.

Out of curiosity, do you know where this stuff is mentioned in the C standards?

[zuiderkwast]

Found it: https://en.cppreference.com/w/c/language/conversion.html#Integer_conversions

[Fusl]

That explicit cast specifically tells clang-tidy "i know what im doing, please stop complaining", there's no other side effect and the explicit and implicit format both get compiled down to the same byte code.

[zuiderkwast]

Would it make sense to add bitfield 0 set i64 0 1 to some test case just to verify the fix?

I tried adding a test case for this but couldn't trigger it outside of a binary built with afl-clang, maybe I was doing something wrong and didn't supply correct compilation flags when building a custom non-afl binary?

Ah, I don't know. We can skip the test.