time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.1k stars 277 forks source link

Arithmetic crash occurs #625

Closed HeeillWang closed 1 year ago

HeeillWang commented 1 year ago

I executed fuzzing, and found arithmetic overflow bug on date.rs. Tested on 0.3.22 but would be reproduced on latest version. Here I omits detailed reproduction steps, as those cases looks quite obvious... Please let me know anyone needs more details.

Case-1 : NumericalStdDuration impl for u64

Thread '<unnamed>' panicked at 'attempt to multiply with overflow', time-0.3.22/src/ext.rs:223
Thread '<unnamed>' panicked at 'attempt to multiply with overflow', time-0.3.22/src/ext.rs:227
Thread '<unnamed>' panicked at 'attempt to multiply with overflow', time-0.3.22/src/ext.rs:231
Thread '<unnamed>' panicked at 'attempt to multiply with overflow', time-0.3.22/src/ext.rs:235

https://github.com/time-rs/time/blob/c96bb1a4474b9af1289edbdf34514fbfe95fa833/time/src/ext.rs#L222-L236

Case-2

Thread '<unnamed>' panicked at 'attempt to divide by zero', time-0.3.22/src/duration.rs:1294

https://github.com/time-rs/time/blob/c96bb1a4474b9af1289edbdf34514fbfe95fa833/time/src/duration.rs#L1348-L1379

Expected patch

jhpratt commented 1 year ago

Case 1 results in wrapping in release mode, which is an issue. I pushed a commit to avoid this by using checked multiplication.

Case 2 is the same as #623, as panicking on division by zero is very much intentional.

Closing as case 1 has been resolved.