Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arithmetic crash occurs #625

Closed
HeeillWang opened this issue Sep 24, 2023 · 1 comment
Closed

Arithmetic crash occurs #625

HeeillWang opened this issue Sep 24, 2023 · 1 comment
Labels
A-core Area: anything not otherwise covered C-bug Category: bug in current code

Comments

@HeeillWang
Copy link

HeeillWang commented Sep 24, 2023

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

time/time/src/ext.rs

Lines 222 to 236 in c96bb1a

fn std_minutes(self) -> StdDuration {
StdDuration::from_secs(self * Second::per(Minute) as Self)
}
fn std_hours(self) -> StdDuration {
StdDuration::from_secs(self * Second::per(Hour) as Self)
}
fn std_days(self) -> StdDuration {
StdDuration::from_secs(self * Second::per(Day) as Self)
}
fn std_weeks(self) -> StdDuration {
StdDuration::from_secs(self * Second::per(Week) as Self)
}

Case-2

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

time/time/src/duration.rs

Lines 1348 to 1379 in c96bb1a

macro_rules! duration_mul_div_int {
($($type:ty),+) => {$(
impl Mul<$type> for Duration {
type Output = Self;
fn mul(self, rhs: $type) -> Self::Output {
Self::nanoseconds_i128(
self.whole_nanoseconds()
.checked_mul(rhs as _)
.expect("overflow when multiplying duration")
)
}
}
impl Mul<Duration> for $type {
type Output = Duration;
fn mul(self, rhs: Duration) -> Self::Output {
rhs * self
}
}
impl Div<$type> for Duration {
type Output = Self;
fn div(self, rhs: $type) -> Self::Output {
Self::nanoseconds_i128(self.whole_nanoseconds() / rhs as i128)
}
}
)+};
}
duration_mul_div_int![i8, i16, i32, u8, u16, u32];

Expected patch

  • Add checking logic with assert statement or checked operations
  • Explicitly mention panic condition on docs.
@jhpratt
Copy link
Member

jhpratt commented Sep 25, 2023

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.

@jhpratt jhpratt closed this as completed Sep 25, 2023
@jhpratt jhpratt added C-bug Category: bug in current code A-core Area: anything not otherwise covered labels Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: anything not otherwise covered C-bug Category: bug in current code
Projects
None yet
Development

No branches or pull requests

2 participants