Skip to content

Commit 05f176f

Browse files
committed
Reject pre-epoch LSPSDateTime at parse time
`LSPSDateTime::is_past` coerced `chrono`'s `i64` timestamp into a `u64` via `try_into().expect(...)`. Because `LSPSDateTime` is parsed from peer-controlled RFC 3339 strings (which can be pre-1970 and so yield negative timestamps), this could be triggered remotely: an attacker-supplied `valid_until` / `expires_at` field of e.g. `"1900-01-01T00:00:00Z"` would parse successfully, land in LSPS state before any HMAC / promise check, and panic the LSP thread on the next `prune_pending_requests` sweep. Concretely reachable today via LSPS2 `opening_fee_params.valid_until` (in the buy request) and the LSPS1 expiry fields. Make `LSPSDateTime::from_str` reject pre-epoch datetimes, and route serde deserialization through it: `#[serde(transparent)]` was delegating Deserialize directly to `chrono`'s impl and bypassing our parser, so peer JSON had to be guarded separately. With both paths funnelled through one parser, no `LSPSDateTime` value with a negative inner timestamp can be constructed and `is_past` is safe by construction. Co-Authored-By: HAL 9000
1 parent 1a26867 commit 05f176f

1 file changed

Lines changed: 30 additions & 3 deletions

File tree

  • lightning-liquidity/src/lsps0

lightning-liquidity/src/lsps0/ser.rs

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ impl Readable for LSPSRequestId {
234234
}
235235

236236
/// An object representing datetimes as described in bLIP-50 / LSPS0.
237-
#[derive(Clone, Debug, Copy, PartialEq, Eq, Hash, Deserialize, Serialize)]
237+
#[derive(Clone, Debug, Copy, PartialEq, Eq, Hash, Serialize)]
238238
#[serde(transparent)]
239239
pub struct LSPSDateTime(pub chrono::DateTime<chrono::Utc>);
240240

@@ -271,8 +271,23 @@ impl LSPSDateTime {
271271
impl FromStr for LSPSDateTime {
272272
type Err = ();
273273
fn from_str(s: &str) -> Result<Self, Self::Err> {
274-
let datetime = chrono::DateTime::parse_from_rfc3339(s).map_err(|_| ())?;
275-
Ok(Self(datetime.into()))
274+
let datetime: chrono::DateTime<chrono::Utc> =
275+
chrono::DateTime::parse_from_rfc3339(s).map_err(|_| ())?.into();
276+
// Reject pre-epoch datetimes here so peer-controlled `valid_until` /
277+
// `expires_at` fields can never produce an `LSPSDateTime` with a negative
278+
// UNIX timestamp, which would otherwise panic the `i64 -> u64` cast in
279+
// `is_past`.
280+
if datetime.timestamp() < 0 {
281+
return Err(());
282+
}
283+
Ok(Self(datetime))
284+
}
285+
}
286+
287+
impl<'de> Deserialize<'de> for LSPSDateTime {
288+
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
289+
let s = String::deserialize(deserializer)?;
290+
Self::from_str(&s).map_err(|()| de::Error::custom("invalid LSPSDateTime"))
276291
}
277292
}
278293

@@ -981,4 +996,16 @@ mod tests {
981996
let decoded_datetime: LSPSDateTime = Readable::read(&mut Cursor::new(buf)).unwrap();
982997
assert_eq!(expected_datetime, decoded_datetime);
983998
}
999+
1000+
#[test]
1001+
#[cfg(feature = "time")]
1002+
fn is_past_handles_pre_epoch_datetime() {
1003+
// A peer-controlled RFC3339 datetime before 1970 must be rejected at parse
1004+
// time, so it can never reach `is_past` (or any other consumer) and panic.
1005+
assert!(LSPSDateTime::from_str("1900-01-01T00:00:00Z").is_err());
1006+
1007+
// JSON deserialization (the path peer messages take) must reject it too.
1008+
let json = "\"1900-01-01T00:00:00Z\"";
1009+
assert!(serde_json::from_str::<LSPSDateTime>(json).is_err());
1010+
}
9841011
}

0 commit comments

Comments
 (0)