Skip to content

Commit 6d40b72

Browse files
sgeislersgeisler
committed
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<Duration> 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<SystemTime> 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.
1 parent e8aef7c commit 6d40b72

File tree

6 files changed

+92
-22
lines changed

6 files changed

+92
-22
lines changed

‎src/libstd/sys/cloudabi/time.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@ pub struct Instant {
1919
t: abi::timestamp,
2020
}
2121

22-
pub fn dur2intervals(dur: &Duration) -> abi::timestamp {
22+
fn checked_dur2intervals(dur: &Duration) -> Option<abi::timestamp> {
2323
dur.as_secs()
2424
.checked_mul(NSEC_PER_SEC)
2525
.and_then(|nanos| nanos.checked_add(dur.subsec_nanos() as abi::timestamp))
26+
}
27+
28+
pub fn dur2intervals(dur: &Duration) -> abi::timestamp {
29+
checked_dur2intervals(dur)
2630
.expect("overflow converting duration to nanoseconds")
2731
}
2832

@@ -92,11 +96,14 @@ impl SystemTime {
9296
}
9397

9498
pub fn add_duration(&self, other: &Duration) -> SystemTime {
95-
SystemTime {
96-
t: self.t
97-
.checked_add(dur2intervals(other))
98-
.expect("overflow when adding duration to instant"),
99-
}
99+
self.checked_add_duration(other)
100+
.expect("overflow when adding duration to instant")
101+
}
102+
103+
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
104+
checked_dur2intervals(other)
105+
.and_then(|d| self.t.checked_add(d))
106+
.map(|t| SystemTime {t})
100107
}
101108

102109
pub fn sub_duration(&self, other: &Duration) -> SystemTime {

‎src/libstd/sys/redox/time.rs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,27 +42,36 @@ impl Timespec {
4242
}
4343

4444
fn add_duration(&self, other: &Duration) -> Timespec {
45-
let mut secs = other
45+
self.checked_add_duration(other).expect("overflow when adding duration to time")
46+
}
47+
48+
fn checked_add_duration(&self, other: &Duration) -> Option<Timespec> {
49+
let mut secs = match other
4650
.as_secs()
4751
.try_into() // <- target type would be `i64`
4852
.ok()
4953
.and_then(|secs| self.t.tv_sec.checked_add(secs))
50-
.expect("overflow when adding duration to time");
54+
{
55+
Some(ts) => ts,
56+
None => return None,
57+
};
5158

5259
// Nano calculations can't overflow because nanos are <1B which fit
5360
// in a u32.
5461
let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32;
5562
if nsec >= NSEC_PER_SEC as u32 {
5663
nsec -= NSEC_PER_SEC as u32;
57-
secs = secs.checked_add(1).expect("overflow when adding \
58-
duration to time");
64+
secs = match secs.checked_add(1) {
65+
Some(ts) => ts,
66+
None => return None,
67+
}
5968
}
60-
Timespec {
69+
Some(Timespec {
6170
t: syscall::TimeSpec {
6271
tv_sec: secs,
6372
tv_nsec: nsec as i32,
6473
},
65-
}
74+
})
6675
}
6776

6877
fn sub_duration(&self, other: &Duration) -> Timespec {
@@ -180,6 +189,10 @@ impl SystemTime {
180189
SystemTime { t: self.t.add_duration(other) }
181190
}
182191

192+
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
193+
self.t.checked_add_duration(other).map(|t| SystemTime { t })
194+
}
195+
183196
pub fn sub_duration(&self, other: &Duration) -> SystemTime {
184197
SystemTime { t: self.t.sub_duration(other) }
185198
}

‎src/libstd/sys/unix/time.rs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,27 +43,36 @@ impl Timespec {
4343
}
4444

4545
fn add_duration(&self, other: &Duration) -> Timespec {
46-
let mut secs = other
46+
self.checked_add_duration(other).expect("overflow when adding duration to time")
47+
}
48+
49+
fn checked_add_duration(&self, other: &Duration) -> Option<Timespec> {
50+
let mut secs = match other
4751
.as_secs()
4852
.try_into() // <- target type would be `libc::time_t`
4953
.ok()
5054
.and_then(|secs| self.t.tv_sec.checked_add(secs))
51-
.expect("overflow when adding duration to time");
55+
{
56+
Some(ts) => ts,
57+
None => return None,
58+
};
5259

5360
// Nano calculations can't overflow because nanos are <1B which fit
5461
// in a u32.
5562
let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32;
5663
if nsec >= NSEC_PER_SEC as u32 {
5764
nsec -= NSEC_PER_SEC as u32;
58-
secs = secs.checked_add(1).expect("overflow when adding \
59-
duration to time");
65+
secs = match secs.checked_add(1) {
66+
Some(ts) => ts,
67+
None => return None,
68+
}
6069
}
61-
Timespec {
70+
Some(Timespec {
6271
t: libc::timespec {
6372
tv_sec: secs,
6473
tv_nsec: nsec as _,
6574
},
66-
}
75+
})
6776
}
6877

6978
fn sub_duration(&self, other: &Duration) -> Timespec {
@@ -201,6 +210,10 @@ mod inner {
201210
SystemTime { t: self.t.add_duration(other) }
202211
}
203212

213+
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
214+
self.t.checked_add_duration(other).map(|t| SystemTime { t })
215+
}
216+
204217
pub fn sub_duration(&self, other: &Duration) -> SystemTime {
205218
SystemTime { t: self.t.sub_duration(other) }
206219
}
@@ -325,6 +338,10 @@ mod inner {
325338
SystemTime { t: self.t.add_duration(other) }
326339
}
327340

341+
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
342+
self.t.checked_add_duration(other).map(|t| SystemTime { t })
343+
}
344+
328345
pub fn sub_duration(&self, other: &Duration) -> SystemTime {
329346
SystemTime { t: self.t.sub_duration(other) }
330347
}

‎src/libstd/sys/wasm/time.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ impl SystemTime {
5151
SystemTime(self.0 + *other)
5252
}
5353

54+
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
55+
self.0.checked_add(*other).map(|d| SystemTime(d))
56+
}
57+
5458
pub fn sub_duration(&self, other: &Duration) -> SystemTime {
5559
SystemTime(self.0 - *other)
5660
}

‎src/libstd/sys/windows/time.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,13 @@ impl SystemTime {
128128
}
129129

130130
pub fn add_duration(&self, other: &Duration) -> SystemTime {
131-
let intervals = self.intervals().checked_add(dur2intervals(other))
132-
.expect("overflow when adding duration to time");
133-
SystemTime::from_intervals(intervals)
131+
self.checked_add_duration(other).expect("overflow when adding duration to time")
132+
}
133+
134+
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
135+
checked_dur2intervals(other)
136+
.and_then(|d| self.intervals().checked_add(d))
137+
.map(|i| SystemTime::from_intervals(i))
134138
}
135139

136140
pub fn sub_duration(&self, other: &Duration) -> SystemTime {
@@ -180,11 +184,15 @@ impl Hash for SystemTime {
180184
}
181185
}
182186

183-
fn dur2intervals(d: &Duration) -> i64 {
187+
fn checked_dur2intervals(d: &Duration) -> Option<i64> {
184188
d.as_secs()
185189
.checked_mul(INTERVALS_PER_SEC)
186190
.and_then(|i| i.checked_add(d.subsec_nanos() as u64 / 100))
187191
.and_then(|i| i.try_into().ok())
192+
}
193+
194+
fn dur2intervals(d: &Duration) -> i64 {
195+
checked_dur2intervals(d)
188196
.expect("overflow when converting duration to intervals")
189197
}
190198

‎src/libstd/time.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,14 @@ impl SystemTime {
357357
pub fn elapsed(&self) -> Result<Duration, SystemTimeError> {
358358
SystemTime::now().duration_since(*self)
359359
}
360+
361+
/// Returns `Some(t)` where `t` is the time `self + duration` if `t` can be represented as
362+
/// `SystemTime` (which means it's inside the bounds of the underlying data structure), `None`
363+
/// otherwise.
364+
#[stable(feature = "time_checked_add", since = "1.32.0")]
365+
pub fn checked_add_duration(&self, duration: &Duration) -> Option<SystemTime> {
366+
self.0.checked_add_duration(duration).map(|t| SystemTime(t))
367+
}
360368
}
361369

362370
#[stable(feature = "time2", since = "1.8.0")]
@@ -557,6 +565,19 @@ mod tests {
557565
let one_second_from_epoch2 = UNIX_EPOCH + Duration::new(0, 500_000_000)
558566
+ Duration::new(0, 500_000_000);
559567
assert_eq!(one_second_from_epoch, one_second_from_epoch2);
568+
569+
// checked_add_duration will not panic on overflow
570+
let mut maybe_t = Some(SystemTime::UNIX_EPOCH);
571+
let max_duration = Duration::from_secs(u64::max_value());
572+
// in case `SystemTime` can store `>= UNIX_EPOCH + max_duration`.
573+
for _ in 0..2 {
574+
maybe_t = maybe_t.and_then(|t| t.checked_add_duration(&max_duration));
575+
}
576+
assert_eq!(maybe_t, None);
577+
578+
// checked_add_duration calculates the right time and will work for another year
579+
let year = Duration::from_secs(60 * 60 * 24 * 365);
580+
assert_eq!(a + year, a.checked_add_duration(&year).unwrap());
560581
}
561582

562583
#[test]

0 commit comments

Comments
 (0)