Skip to content

Commit

Permalink
Merge pull request #1997 from cyang1/systemtime-panics
Browse files Browse the repository at this point in the history
Prevent various panics when deserializing malformed SystemTime
  • Loading branch information
dtolnay authored Mar 6, 2021
2 parents c261015 + 4118cec commit d91075c
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 3 deletions.
4 changes: 3 additions & 1 deletion serde/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,14 @@ fn main() {
println!("cargo:rustc-cfg=serde_derive");
}

// TryFrom, Atomic types, and non-zero signed integers stabilized in Rust 1.34:
// TryFrom, Atomic types, non-zero signed integers, and `SystemTime::checked_add`
// stabilized in Rust 1.34:
// https://blog.rust-lang.org/2019/04/11/Rust-1.34.0.html#tryfrom-and-tryinto
// https://blog.rust-lang.org/2019/04/11/Rust-1.34.0.html#library-stabilizations
if minor >= 34 {
println!("cargo:rustc-cfg=core_try_from");
println!("cargo:rustc-cfg=num_nonzero_signed");
println!("cargo:rustc-cfg=systemtime_checked_add");

// Whitelist of archs that support std::sync::atomic module. Ideally we
// would use #[cfg(target_has_atomic = "...")] but it is not stable yet.
Expand Down
21 changes: 20 additions & 1 deletion serde/src/de/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2046,6 +2046,17 @@ impl<'de> Deserialize<'de> for SystemTime {
}
}

fn check_overflow<E>(secs: u64, nanos: u32) -> Result<(), E>
where
E: Error,
{
static NANOS_PER_SEC: u32 = 1_000_000_000;
match secs.checked_add((nanos / NANOS_PER_SEC) as u64) {
Some(_) => Ok(()),
None => Err(E::custom("overflow deserializing SystemTime epoch offset")),
}
}

struct DurationVisitor;

impl<'de> Visitor<'de> for DurationVisitor {
Expand All @@ -2071,6 +2082,7 @@ impl<'de> Deserialize<'de> for SystemTime {
return Err(Error::invalid_length(1, &self));
}
};
try!(check_overflow(secs, nanos));
Ok(Duration::new(secs, nanos))
}

Expand Down Expand Up @@ -2108,13 +2120,20 @@ impl<'de> Deserialize<'de> for SystemTime {
Some(nanos) => nanos,
None => return Err(<A::Error as Error>::missing_field("nanos_since_epoch")),
};
try!(check_overflow(secs, nanos));
Ok(Duration::new(secs, nanos))
}
}

const FIELDS: &'static [&'static str] = &["secs_since_epoch", "nanos_since_epoch"];
let duration = try!(deserializer.deserialize_struct("SystemTime", FIELDS, DurationVisitor));
Ok(UNIX_EPOCH + duration)
#[cfg(systemtime_checked_add)]
let ret = UNIX_EPOCH
.checked_add(duration)
.ok_or(D::Error::custom("overflow deserializing SystemTime"));
#[cfg(not(systemtime_checked_add))]
let ret = Ok(UNIX_EPOCH + duration);
ret
}
}

Expand Down
46 changes: 45 additions & 1 deletion test_suite/tests/test_de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::sync::atomic::{
AtomicUsize, Ordering,
};
use std::sync::{Arc, Weak as ArcWeak};
use std::time::{Duration, UNIX_EPOCH};
use std::time::{Duration, SystemTime, UNIX_EPOCH};

#[cfg(target_arch = "x86_64")]
use std::sync::atomic::{AtomicI64, AtomicU64};
Expand Down Expand Up @@ -199,6 +199,19 @@ macro_rules! declare_error_tests {
assert_de_tokens_error::<$target>($tokens, $expected);
}
)+
};

($(
$(#[$cfg:meta])*
$name:ident<$target:ty> { $tokens:expr, $expected:expr, }
)+) => {
$(
$(#[$cfg])*
#[test]
fn $name() {
assert_de_tokens_error::<$target>($tokens, $expected);
}
)+
}
}

Expand Down Expand Up @@ -1614,4 +1627,35 @@ declare_error_tests! {
],
"overflow deserializing Duration",
}
test_systemtime_overflow_seq<SystemTime> {
&[
Token::Seq { len: Some(2) },
Token::U64(u64::max_value()),
Token::U32(1_000_000_000),
Token::SeqEnd,
],
"overflow deserializing SystemTime epoch offset",
}
test_systemtime_overflow_struct<SystemTime> {
&[
Token::Struct { name: "SystemTime", len: 2 },
Token::Str("secs_since_epoch"),
Token::U64(u64::max_value()),

Token::Str("nanos_since_epoch"),
Token::U32(1_000_000_000),
Token::StructEnd,
],
"overflow deserializing SystemTime epoch offset",
}
#[cfg(systemtime_checked_add)]
test_systemtime_overflow<SystemTime> {
&[
Token::Seq { len: Some(2) },
Token::U64(u64::max_value()),
Token::U32(0),
Token::SeqEnd,
],
"overflow deserializing SystemTime",
}
}

0 comments on commit d91075c

Please sign in to comment.