search

bitsmanent / snore

sleep with feedback
MIT License
73 stars 11 forks source link

Signed integer overflow #17

Closed jpdoyle closed 2 years ago

jpdoyle commented 2 years ago

I added undefined behavior sanitization to snore in a small patch: https://github.com/jpdoyle/snore/tree/ubsan-overflow

With that patch, I can trigger signed integer overflow by doing:

$ ./snore 4294967294.5
snore.c:100:20: runtime error: signed integer overflow: 49710 * 86400 cannot be represented in type 'int'
bitsmanent commented 2 years ago

Thanks for your feedback.

I would rather prevent the overflow by changing line 122 to INT_MAX. Do you agree? If so please send a PR so that I can merge your fix.

jpdoyle commented 2 years ago

Would you prefer to not have ubsan enabled? It seems like a good thing to include, especially since print_time will have undefined behavior for large numbers on any platform where int is 64-bit.

I can make a PR with both ubsan and the UINT_MAX -> INT_MAX change

bitsmanent commented 2 years ago

Checking against INT_MAX should prevent undefined behaviours but I will give a deeper look and make some test.

I will merge #18 for now.