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.

diff --git a/src/libstd/sys/cloudabi/time.rs b/src/libstd/sys/cloudabi/time.rs
index ee12731619aac3bfbae63a38a796ba8984a7ec4d..191324e26a40f7d0bc79d746d45e75496de7119d 100644 (file)
--- 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<abi::timestamp> {
     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 @@ pub fn sub_time(&self, other: &SystemTime) -> Result<Duration, Duration> {
     }
 
     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<SystemTime> {
+        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 5c491115c55160dafcc356c2082699454b9b29f4..5f8799489aa6a42005fe1f82d0bde907d218acb1 100644 (file)
--- a/src/libstd/sys/redox/time.rs
+++ b/src/libstd/sys/redox/time.rs
@@ -42,27 +42,36 @@ fn sub_timespec(&self, other: &Timespec) -> Result<Duration, Duration> {
     }
 
     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<Timespec> {
+        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 @@ pub fn add_duration(&self, other: &Duration) -> SystemTime {
         SystemTime { t: self.t.add_duration(other) }
     }
 
+    pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
+        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 0b1fb726357e1056d8685ce50e338843d842c5a8..50c3c00382eb5053e7e7e7d9dfad5642137d0edb 100644 (file)
--- a/src/libstd/sys/unix/time.rs
+++ b/src/libstd/sys/unix/time.rs
@@ -43,27 +43,36 @@ fn sub_timespec(&self, other: &Timespec) -> Result<Duration, Duration> {
     }
 
     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<Timespec> {
+        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 @@ pub fn add_duration(&self, other: &Duration) -> SystemTime {
             SystemTime { t: self.t.add_duration(other) }
         }
 
+        pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
+            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 @@ pub fn add_duration(&self, other: &Duration) -> SystemTime {
             SystemTime { t: self.t.add_duration(other) }
         }
 
+        pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
+            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 e52435e63398f226e6cb735a431012f7eb0b3350..991e8176edf6d8182bd2a70669549301c933f7f9 100644 (file)
--- a/src/libstd/sys/wasm/time.rs
+++ b/src/libstd/sys/wasm/time.rs
@@ -51,6 +51,10 @@ pub fn add_duration(&self, other: &Duration) -> SystemTime {
         SystemTime(self.0 + *other)
     }
 
+    pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
+        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 07e64d386a1c2b7c94927fda6d73a1f6b190ba5e..8eebe4a85fe160648d85e0d34509ca8345e3e01c 100644 (file)
--- a/src/libstd/sys/windows/time.rs
+++ b/src/libstd/sys/windows/time.rs
@@ -128,9 +128,13 @@ pub fn sub_time(&self, other: &SystemTime) -> Result<Duration, Duration> {
     }
 
     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<SystemTime> {
+        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 @@ fn hash<H : Hasher>(&self, state: &mut H) {
     }
 }
 
-fn dur2intervals(d: &Duration) -> i64 {
+fn checked_dur2intervals(d: &Duration) -> Option<i64> {
     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 90ab349159915cad86c8a10d386558cc3a729cce..94875d8993d335cc125cf2e2a8007456e6ab6335 100644 (file)
--- a/src/libstd/time.rs
+++ b/src/libstd/time.rs
@@ -357,6 +357,14 @@ pub fn duration_since(&self, earlier: SystemTime)
     pub fn elapsed(&self) -> Result<Duration, SystemTimeError> {
         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<SystemTime> {
+        self.0.checked_add_duration(duration).map(|t| SystemTime(t))
+    }
 }
 
 #[stable(feature = "time2", since = "1.8.0")]
@@ -557,6 +565,19 @@ fn system_time_math() {
         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]