From a38b59da84774c02a1cca06f73bcc15530aa8bc6 Mon Sep 17 00:00:00 2001 From: oxalica Date: Tue, 24 Sep 2024 20:36:27 -0400 Subject: [PATCH] fix(types,blahd): reject timestamps > i64::MAX --- blah-types/src/identity.rs | 18 ++++++++++++++++++ blahd/config.example.toml | 2 ++ blahd/src/database.rs | 5 ++--- blahd/src/lib.rs | 12 ++++-------- blahd/tests/webapi.rs | 2 +- 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/blah-types/src/identity.rs b/blah-types/src/identity.rs index 741385e..4091451 100644 --- a/blah-types/src/identity.rs +++ b/blah-types/src/identity.rs @@ -29,6 +29,8 @@ pub struct VerifyError(#[from] VerifyErrorImpl); enum VerifyErrorImpl { #[error("profile id_key mismatch")] ProfileIdKeyMismatch, + #[error("act_key[{0}] has invalid expiring timestamp")] + ActKeyTimestamp(usize), #[error("act_key[{0}] not signed by id_key")] ActKeySigner(usize), #[error("invalid act_key[{0}] signature: {1}")] @@ -66,6 +68,9 @@ impl UserIdentityDesc { { return Err(VerifyErrorImpl::ActKeySigner(i).into()); } + if i64::try_from(kdesc.expire_time).is_err() { + return Err(VerifyErrorImpl::ActKeyTimestamp(i).into()); + } signed_kdesc .verify() .map_err(|err| VerifyErrorImpl::ActKeySignature(i, err))?; @@ -335,6 +340,19 @@ mod tests { VerifyErrorImpl::ActKeySigner(1), ); + // Timestamp overflows i64. + id_desc.act_keys[1] = UserActKeyDesc { + act_key: act_pub.clone(), + expire_time: u64::MAX, + comment: String::new(), + } + .sign_msg_with(&id_key, &id_priv, TIMESTAMP, rng) + .unwrap(); + assert_err!( + id_desc.verify(None, TIMESTAMP), + VerifyErrorImpl::ActKeyTimestamp(1), + ); + // OK act key. id_desc.act_keys[1] = UserActKeyDesc { act_key: act_pub.clone(), diff --git a/blahd/config.example.toml b/blahd/config.example.toml index c4e04e8..b34492b 100644 --- a/blahd/config.example.toml +++ b/blahd/config.example.toml @@ -41,6 +41,8 @@ max_page_len = 1024 max_request_len = 4096 # The maximum timestamp tolerance in seconds for request validation. +# NB. This should be small enough to reject timestamps overflowing `i64`, +# otherwise it may cause panics. timestamp_tolerance_secs = 90 [server.feed] diff --git a/blahd/src/database.rs b/blahd/src/database.rs index ba2a7a8..3310366 100644 --- a/blahd/src/database.rs +++ b/blahd/src/database.rs @@ -432,8 +432,7 @@ pub trait TransactionOps { stmt.execute(named_params! { ":uid": uid, ":act_key": kdesc.signee.payload.act_key, - // FIXME: Other `u64` that will be stored in database should also be range checked. - ":expire_time": kdesc.signee.payload.expire_time.min(i64::MAX as _), + ":expire_time": i64::try_from(kdesc.signee.payload.expire_time).expect("verified timestamp"), })?; } @@ -537,7 +536,7 @@ pub trait TransactionOps { fn add_room_chat_msg(&self, rid: Id, uid: i64, cid: Id, chat: &SignedChatMsg) -> Result<()> { let conn = self.conn(); let act_key = &chat.signee.user.act_key; - let timestamp = chat.signee.timestamp; + let timestamp = i64::try_from(chat.signee.timestamp).expect("verified timestamp"); let nonce = chat.signee.nonce; let rich_text = &chat.signee.payload.rich_text; let sig = &chat.sig; diff --git a/blahd/src/lib.rs b/blahd/src/lib.rs index 712a027..2c34a1b 100644 --- a/blahd/src/lib.rs +++ b/blahd/src/lib.rs @@ -1,6 +1,6 @@ use std::num::NonZero; use std::sync::Arc; -use std::time::{Duration, SystemTime}; +use std::time::Duration; use anyhow::Result; use axum::extract::{ws, OriginalUri}; @@ -11,8 +11,8 @@ use axum::routing::{get, post}; use axum::{Json, Router}; use axum_extra::extract::WithRejection as R; use blah_types::{ - ChatPayload, CreateGroup, CreatePeerChat, CreateRoomPayload, DeleteRoomPayload, Id, - MemberPermission, RoomAdminOp, RoomAdminPayload, RoomAttrs, RoomMetadata, ServerPermission, + get_timestamp, ChatPayload, CreateGroup, CreatePeerChat, CreateRoomPayload, DeleteRoomPayload, + Id, MemberPermission, RoomAdminOp, RoomAdminPayload, RoomAttrs, RoomMetadata, ServerPermission, Signed, SignedChatMsg, UserKey, UserRegisterPayload, WithMsgId, X_BLAH_DIFFICULTY, X_BLAH_NONCE, }; @@ -103,11 +103,7 @@ impl AppState { fn verify_signed_data(&self, data: &Signed) -> Result<(), ApiError> { api_ensure!(data.verify().is_ok(), "signature verification failed"); - let timestamp_diff = SystemTime::now() - .duration_since(SystemTime::UNIX_EPOCH) - .expect("after UNIX epoch") - .as_secs() - .abs_diff(data.signee.timestamp); + let timestamp_diff = get_timestamp().abs_diff(data.signee.timestamp); api_ensure!( timestamp_diff <= self.config.timestamp_tolerance_secs, "invalid timestamp", diff --git a/blahd/tests/webapi.rs b/blahd/tests/webapi.rs index 7682f84..f2f52d2 100644 --- a/blahd/tests/webapi.rs +++ b/blahd/tests/webapi.rs @@ -1190,7 +1190,7 @@ async fn register_flow(server: Server) { // Sign using id_key. let act_key = UserActKeyDesc { act_key: CAROL.pubkeys.act_key.clone(), - expire_time: u64::MAX, + expire_time: i64::MAX as _, comment: "comment".into(), } .sign_msg(&CAROL.pubkeys.id_key, &CAROL.id_priv)