From 6d40b7232eaa00ab5c060582011f350725703a1e Mon Sep 17 00:00:00 2001 From: Sebastian Geisler Date: Tue, 30 Oct 2018 22:24:33 -0700 Subject: [PATCH] Implement checked_add_duration for SystemTime Since SystemTime is opaque there is no way to check if the result of an addition will be in bounds. That makes the Add trait completely unusable with untrusted data. This is a big problem because adding a Duration to UNIX_EPOCH is the standard way of constructing a SystemTime from a unix timestamp. This commit implements checked_add_duration(&self, &Duration) -> Option for std::time::SystemTime and as a prerequisite also for all platform specific time structs. This also led to the refactoring of many add_duration(&self, &Duration) -> SystemTime functions to avoid redundancy (they now unwrap the result of checked_add_duration). Some basic unit tests for the newly introduced function were added too. --- src/libstd/sys/cloudabi/time.rs | 19 +++++++++++++------ src/libstd/sys/redox/time.rs | 25 +++++++++++++++++++------ src/libstd/sys/unix/time.rs | 29 +++++++++++++++++++++++------ src/libstd/sys/wasm/time.rs | 4 ++++ src/libstd/sys/windows/time.rs | 16 ++++++++++++---- src/libstd/time.rs | 21 +++++++++++++++++++++ 6 files changed, 92 insertions(+), 22 deletions(-) diff --git a/src/libstd/sys/cloudabi/time.rs b/src/libstd/sys/cloudabi/time.rs index ee12731619a..191324e26a4 100644 --- a/src/libstd/sys/cloudabi/time.rs +++ b/src/libstd/sys/cloudabi/time.rs @@ -19,10 +19,14 @@ pub struct Instant { t: abi::timestamp, } -pub fn dur2intervals(dur: &Duration) -> abi::timestamp { +fn checked_dur2intervals(dur: &Duration) -> Option { dur.as_secs() .checked_mul(NSEC_PER_SEC) .and_then(|nanos| nanos.checked_add(dur.subsec_nanos() as abi::timestamp)) +} + +pub fn dur2intervals(dur: &Duration) -> abi::timestamp { + checked_dur2intervals(dur) .expect("overflow converting duration to nanoseconds") } @@ -92,11 +96,14 @@ impl SystemTime { } pub fn add_duration(&self, other: &Duration) -> SystemTime { - SystemTime { - t: self.t - .checked_add(dur2intervals(other)) - .expect("overflow when adding duration to instant"), - } + self.checked_add_duration(other) + .expect("overflow when adding duration to instant") + } + + pub fn checked_add_duration(&self, other: &Duration) -> Option { + checked_dur2intervals(other) + .and_then(|d| self.t.checked_add(d)) + .map(|t| SystemTime {t}) } pub fn sub_duration(&self, other: &Duration) -> SystemTime { diff --git a/src/libstd/sys/redox/time.rs b/src/libstd/sys/redox/time.rs index 5c491115c55..5f8799489aa 100644 --- a/src/libstd/sys/redox/time.rs +++ b/src/libstd/sys/redox/time.rs @@ -42,27 +42,36 @@ impl Timespec { } fn add_duration(&self, other: &Duration) -> Timespec { - let mut secs = other + self.checked_add_duration(other).expect("overflow when adding duration to time") + } + + fn checked_add_duration(&self, other: &Duration) -> Option { + let mut secs = match other .as_secs() .try_into() // <- target type would be `i64` .ok() .and_then(|secs| self.t.tv_sec.checked_add(secs)) - .expect("overflow when adding duration to time"); + { + Some(ts) => ts, + None => return None, + }; // Nano calculations can't overflow because nanos are <1B which fit // in a u32. let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32; if nsec >= NSEC_PER_SEC as u32 { nsec -= NSEC_PER_SEC as u32; - secs = secs.checked_add(1).expect("overflow when adding \ - duration to time"); + secs = match secs.checked_add(1) { + Some(ts) => ts, + None => return None, + } } - Timespec { + Some(Timespec { t: syscall::TimeSpec { tv_sec: secs, tv_nsec: nsec as i32, }, - } + }) } fn sub_duration(&self, other: &Duration) -> Timespec { @@ -180,6 +189,10 @@ impl SystemTime { SystemTime { t: self.t.add_duration(other) } } + pub fn checked_add_duration(&self, other: &Duration) -> Option { + self.t.checked_add_duration(other).map(|t| SystemTime { t }) + } + pub fn sub_duration(&self, other: &Duration) -> SystemTime { SystemTime { t: self.t.sub_duration(other) } } diff --git a/src/libstd/sys/unix/time.rs b/src/libstd/sys/unix/time.rs index 0b1fb726357..50c3c00382e 100644 --- a/src/libstd/sys/unix/time.rs +++ b/src/libstd/sys/unix/time.rs @@ -43,27 +43,36 @@ impl Timespec { } fn add_duration(&self, other: &Duration) -> Timespec { - let mut secs = other + self.checked_add_duration(other).expect("overflow when adding duration to time") + } + + fn checked_add_duration(&self, other: &Duration) -> Option { + let mut secs = match other .as_secs() .try_into() // <- target type would be `libc::time_t` .ok() .and_then(|secs| self.t.tv_sec.checked_add(secs)) - .expect("overflow when adding duration to time"); + { + Some(ts) => ts, + None => return None, + }; // Nano calculations can't overflow because nanos are <1B which fit // in a u32. let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32; if nsec >= NSEC_PER_SEC as u32 { nsec -= NSEC_PER_SEC as u32; - secs = secs.checked_add(1).expect("overflow when adding \ - duration to time"); + secs = match secs.checked_add(1) { + Some(ts) => ts, + None => return None, + } } - Timespec { + Some(Timespec { t: libc::timespec { tv_sec: secs, tv_nsec: nsec as _, }, - } + }) } fn sub_duration(&self, other: &Duration) -> Timespec { @@ -201,6 +210,10 @@ mod inner { SystemTime { t: self.t.add_duration(other) } } + pub fn checked_add_duration(&self, other: &Duration) -> Option { + self.t.checked_add_duration(other).map(|t| SystemTime { t }) + } + pub fn sub_duration(&self, other: &Duration) -> SystemTime { SystemTime { t: self.t.sub_duration(other) } } @@ -325,6 +338,10 @@ mod inner { SystemTime { t: self.t.add_duration(other) } } + pub fn checked_add_duration(&self, other: &Duration) -> Option { + self.t.checked_add_duration(other).map(|t| SystemTime { t }) + } + pub fn sub_duration(&self, other: &Duration) -> SystemTime { SystemTime { t: self.t.sub_duration(other) } } diff --git a/src/libstd/sys/wasm/time.rs b/src/libstd/sys/wasm/time.rs index e52435e6339..991e8176edf 100644 --- a/src/libstd/sys/wasm/time.rs +++ b/src/libstd/sys/wasm/time.rs @@ -51,6 +51,10 @@ impl SystemTime { SystemTime(self.0 + *other) } + pub fn checked_add_duration(&self, other: &Duration) -> Option { + self.0.checked_add(*other).map(|d| SystemTime(d)) + } + pub fn sub_duration(&self, other: &Duration) -> SystemTime { SystemTime(self.0 - *other) } diff --git a/src/libstd/sys/windows/time.rs b/src/libstd/sys/windows/time.rs index 07e64d386a1..8eebe4a85fe 100644 --- a/src/libstd/sys/windows/time.rs +++ b/src/libstd/sys/windows/time.rs @@ -128,9 +128,13 @@ impl SystemTime { } pub fn add_duration(&self, other: &Duration) -> SystemTime { - let intervals = self.intervals().checked_add(dur2intervals(other)) - .expect("overflow when adding duration to time"); - SystemTime::from_intervals(intervals) + self.checked_add_duration(other).expect("overflow when adding duration to time") + } + + pub fn checked_add_duration(&self, other: &Duration) -> Option { + checked_dur2intervals(other) + .and_then(|d| self.intervals().checked_add(d)) + .map(|i| SystemTime::from_intervals(i)) } pub fn sub_duration(&self, other: &Duration) -> SystemTime { @@ -180,11 +184,15 @@ impl Hash for SystemTime { } } -fn dur2intervals(d: &Duration) -> i64 { +fn checked_dur2intervals(d: &Duration) -> Option { d.as_secs() .checked_mul(INTERVALS_PER_SEC) .and_then(|i| i.checked_add(d.subsec_nanos() as u64 / 100)) .and_then(|i| i.try_into().ok()) +} + +fn dur2intervals(d: &Duration) -> i64 { + checked_dur2intervals(d) .expect("overflow when converting duration to intervals") } diff --git a/src/libstd/time.rs b/src/libstd/time.rs index 90ab3491599..94875d8993d 100644 --- a/src/libstd/time.rs +++ b/src/libstd/time.rs @@ -357,6 +357,14 @@ impl SystemTime { pub fn elapsed(&self) -> Result { SystemTime::now().duration_since(*self) } + + /// Returns `Some(t)` where `t` is the time `self + duration` if `t` can be represented as + /// `SystemTime` (which means it's inside the bounds of the underlying data structure), `None` + /// otherwise. + #[stable(feature = "time_checked_add", since = "1.32.0")] + pub fn checked_add_duration(&self, duration: &Duration) -> Option { + self.0.checked_add_duration(duration).map(|t| SystemTime(t)) + } } #[stable(feature = "time2", since = "1.8.0")] @@ -557,6 +565,19 @@ mod tests { let one_second_from_epoch2 = UNIX_EPOCH + Duration::new(0, 500_000_000) + Duration::new(0, 500_000_000); assert_eq!(one_second_from_epoch, one_second_from_epoch2); + + // checked_add_duration will not panic on overflow + let mut maybe_t = Some(SystemTime::UNIX_EPOCH); + let max_duration = Duration::from_secs(u64::max_value()); + // in case `SystemTime` can store `>= UNIX_EPOCH + max_duration`. + for _ in 0..2 { + maybe_t = maybe_t.and_then(|t| t.checked_add_duration(&max_duration)); + } + assert_eq!(maybe_t, None); + + // checked_add_duration calculates the right time and will work for another year + let year = Duration::from_secs(60 * 60 * 24 * 365); + assert_eq!(a + year, a.checked_add_duration(&year).unwrap()); } #[test]