From c6af7c0a2d30dd2fb30b6eb5c54b57f68890984f Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Thu, 12 Aug 2021 12:21:48 +0200 Subject: [PATCH] Make sure the end offset doesn't overflow We used to only check that the length was not too big but didn't check that the starting offset wasn't too big. We want the end offset to not overflow. --- src/ctap/large_blobs.rs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/ctap/large_blobs.rs b/src/ctap/large_blobs.rs index 846bc33..9a01b8f 100644 --- a/src/ctap/large_blobs.rs +++ b/src/ctap/large_blobs.rs @@ -62,7 +62,7 @@ impl LargeBlobs { const MAX_FRAGMENT_LENGTH: usize = MAX_MSG_SIZE - 64; if let Some(get) = get { - if get > MAX_FRAGMENT_LENGTH { + if get > MAX_FRAGMENT_LENGTH || offset.checked_add(get).is_none() { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_LENGTH); } let config = persistent_store.get_large_blob_array(offset, get)?; @@ -328,6 +328,30 @@ mod test { ); } + #[test] + fn test_process_command_commit_end_offset_overflow() { + let mut rng = ThreadRng256 {}; + let mut persistent_store = PersistentStore::new(&mut rng); + let key_agreement_key = crypto::ecdh::SecKey::gensk(&mut rng); + let pin_uv_auth_token = [0x55; 32]; + let mut client_pin = + ClientPin::new_test(key_agreement_key, pin_uv_auth_token, PinUvAuthProtocol::V1); + let mut large_blobs = LargeBlobs::new(); + + let large_blobs_params = AuthenticatorLargeBlobsParameters { + get: Some(1), + set: None, + offset: usize::MAX, + length: None, + pin_uv_auth_param: None, + pin_uv_auth_protocol: None, + }; + assert_eq!( + large_blobs.process_command(&mut persistent_store, &mut client_pin, large_blobs_params), + Err(Ctap2StatusCode::CTAP1_ERR_INVALID_LENGTH), + ); + } + #[test] fn test_process_command_commit_unexpected_hash() { let mut rng = ThreadRng256 {};