From 2fc798b94d496e31452628d5f23199c8a0b6e417 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 10 May 2026 21:06:37 +0100 Subject: [PATCH 1/5] chore(clippy): enable indexing_slicing and unwrap_in_result lints `clippy::indexing_slicing` flags any `&v[i]` / `&v[a..b]` that could panic at runtime, complementing the existing `deny(clippy::panic)` which does not see slice-index panics from transitive crates. `clippy::unwrap_in_result` flags `.unwrap()` / `.expect()` inside functions that return `Result`, where bubbling the error is almost always preferable. Enable both for non-test code; tests retain the latitude they already have via the existing cfg_attr pattern. This commit is intentionally lint-failing: subsequent commits in this PR fix the call sites that the new lints surface. --- libwebauthn/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libwebauthn/src/lib.rs b/libwebauthn/src/lib.rs index 1c18e2b..7972f91 100644 --- a/libwebauthn/src/lib.rs +++ b/libwebauthn/src/lib.rs @@ -1,10 +1,11 @@ -// Deny panic-inducing patterns in production code. // Tests and the virt test-utility feature are allowed to use unwrap/expect/panic for convenience. #![cfg_attr(not(any(test, feature = "virt")), deny(clippy::unwrap_used))] #![cfg_attr(not(any(test, feature = "virt")), deny(clippy::expect_used))] #![cfg_attr(not(any(test, feature = "virt")), deny(clippy::panic))] #![cfg_attr(not(any(test, feature = "virt")), deny(clippy::todo))] #![cfg_attr(not(any(test, feature = "virt")), deny(clippy::unreachable))] +#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::indexing_slicing))] +#![cfg_attr(not(any(test, feature = "virt")), deny(clippy::unwrap_in_result))] #[cfg(all( feature = "nfc", From d9ef201f559327b90632d213395e5eb2bc0d7b96 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 10 May 2026 21:13:10 +0100 Subject: [PATCH 2/5] fix(proto): bounds-check slicing flagged by clippy::indexing_slicing Replace direct slice / index access with `.get()`, `split_first`, `split_at`, or iterator-based equivalents in CTAP message parsing and PIN/UV helpers. Functions affected: - `fido::AuthenticatorData::deserialize`: bound the trailing-data check. - `pin::PinUvAuthProtocolOne::authenticate`: handle the (impossible in practice) case of an HMAC output shorter than 16 bytes. - `pin::pin_hash`: use iterator `take(16)` on the SHA-256 output. - `proto::ctap1::apdu::ApduResponse::try_from`: rewrite using `checked_sub` and `split_at` so an empty / 1-byte packet errors out. - `proto::ctap1::model`: bound the U2F register-response signature split via `checked_sub`. - `proto::ctap2::cbor::CborResponse::try_from`: rewrite using `split_first`. - `proto::ctap2::model::get_assertion`: convert SHA-256 outputs to `[u8; 32]` via `GenericArray::into` and use `first()` on the allowList shortcut. --- libwebauthn/src/fido.rs | 6 ++++- libwebauthn/src/pin.rs | 14 ++++++++--- libwebauthn/src/proto/ctap1/apdu/response.rs | 23 ++++++++++++------- libwebauthn/src/proto/ctap1/model.rs | 13 ++++++++++- libwebauthn/src/proto/ctap2/cbor/response.rs | 20 ++++++++-------- .../src/proto/ctap2/model/get_assertion.rs | 13 +++++------ 6 files changed, 59 insertions(+), 30 deletions(-) diff --git a/libwebauthn/src/fido.rs b/libwebauthn/src/fido.rs index 5d22778..f21b46d 100644 --- a/libwebauthn/src/fido.rs +++ b/libwebauthn/src/fido.rs @@ -249,7 +249,11 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for AuthenticatorData { }; // Check if we have trailing data - if !&data[cursor.position() as usize..].is_empty() { + let pos = cursor.position() as usize; + let trailing = data.get(pos..).ok_or_else(|| { + DesError::custom("cursor advanced past end of authenticator data") + })?; + if !trailing.is_empty() { return Err(DesError::invalid_length(data.len(), &"trailing data")); } diff --git a/libwebauthn/src/pin.rs b/libwebauthn/src/pin.rs index 00844f4..3c022fa 100644 --- a/libwebauthn/src/pin.rs +++ b/libwebauthn/src/pin.rs @@ -262,7 +262,14 @@ impl PinUvAuthProtocol for PinUvAuthProtocolOne { fn authenticate(&self, key: &[u8], message: &[u8]) -> Result, Error> { // Return the first 16 bytes of the result of computing HMAC-SHA-256 with the given key and message. let hmac = hmac_sha256(key, message)?; - Ok(Vec::from(&hmac[..16])) + // HMAC-SHA-256 produces 32 bytes, so this slice is always valid. + let truncated = hmac.get(..16).ok_or_else(|| { + error!(len = hmac.len(), "HMAC output shorter than 16 bytes"); + Error::Platform(PlatformError::CryptoError( + "HMAC output shorter than 16 bytes".into(), + )) + })?; + Ok(Vec::from(truncated)) } #[instrument(skip_all)] @@ -438,8 +445,9 @@ impl PinUvAuthProtocol for PinUvAuthProtocolTwo { pub fn pin_hash(pin: &[u8]) -> Vec { let mut hasher = Sha256::default(); hasher.update(pin); - let hashed = hasher.finalize().to_vec(); - Vec::from(&hashed[..16]) + let hashed = hasher.finalize(); + // SHA-256 output is fixed at 32 bytes; keep only the first 16 per the spec. + hashed.into_iter().take(16).collect() } pub fn hmac_sha256(key: &[u8], message: &[u8]) -> Result, Error> { diff --git a/libwebauthn/src/proto/ctap1/apdu/response.rs b/libwebauthn/src/proto/ctap1/apdu/response.rs index 95a9289..e6c77e5 100644 --- a/libwebauthn/src/proto/ctap1/apdu/response.rs +++ b/libwebauthn/src/proto/ctap1/apdu/response.rs @@ -45,19 +45,26 @@ impl ApduResponse { impl TryFrom<&Vec> for ApduResponse { type Error = IOError; fn try_from(packet: &Vec) -> Result { - if packet.len() < 2 { - return Err(IOError::new( + let split_at = packet.len().checked_sub(2).ok_or_else(|| { + IOError::new( IOErrorKind::InvalidData, "Apdu response packets must contain at least 2 bytes.", - )); - } + ) + })?; + let (body, status) = packet.split_at(split_at); + // `status` is guaranteed to have exactly 2 elements by construction above. + let sw1 = *status + .first() + .ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Missing APDU status byte 1"))?; + let sw2 = *status + .get(1) + .ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Missing APDU status byte 2"))?; - let data = if packet.len() > 2 { - Some(Vec::from(&packet[0..packet.len() - 2])) - } else { + let data = if body.is_empty() { None + } else { + Some(Vec::from(body)) }; - let (sw1, sw2) = (packet[packet.len() - 2], packet[packet.len() - 1]); Ok(Self { data, sw1, sw2 }) } diff --git a/libwebauthn/src/proto/ctap1/model.rs b/libwebauthn/src/proto/ctap1/model.rs index 9a6a192..1bc1af5 100644 --- a/libwebauthn/src/proto/ctap1/model.rs +++ b/libwebauthn/src/proto/ctap1/model.rs @@ -142,7 +142,18 @@ impl TryFrom for Ctap1RegisterResponse { "Failed to parse X509 attestation data", )))?; let signature = Vec::from(signature); - let attestation = Vec::from(&remaining[0..remaining.len() - signature.len()]); + // `signature` is a suffix of `remaining` returned by `X509Certificate::from_der`, + // so the split index is always within bounds in practice; bound it explicitly. + let split_at = remaining + .len() + .checked_sub(signature.len()) + .ok_or_else(|| { + IOError::new( + IOErrorKind::InvalidData, + "Signature longer than remaining U2F register response data", + ) + })?; + let attestation = Vec::from(remaining.get(..split_at).unwrap_or(&[])); Ok(Ctap1RegisterResponse { version: Ctap1Version::U2fV2, diff --git a/libwebauthn/src/proto/ctap2/cbor/response.rs b/libwebauthn/src/proto/ctap2/cbor/response.rs index fc1a457..e52970e 100644 --- a/libwebauthn/src/proto/ctap2/cbor/response.rs +++ b/libwebauthn/src/proto/ctap2/cbor/response.rs @@ -25,25 +25,25 @@ impl CborResponse { impl TryFrom<&Vec> for CborResponse { type Error = IOError; fn try_from(packet: &Vec) -> Result { - if packet.is_empty() { - return Err(IOError::new( + let (status_byte, body) = packet.split_first().ok_or_else(|| { + IOError::new( IOErrorKind::InvalidData, "Cbor response packets must contain at least 1 byte.", - )); - } + ) + })?; - let Ok(status_code) = packet[0].try_into() else { - error!({ code = ?packet[0] }, "Invalid CTAP error code"); + let Ok(status_code) = (*status_byte).try_into() else { + error!({ code = ?*status_byte }, "Invalid CTAP error code"); return Err(IOError::new( IOErrorKind::InvalidData, - format!("Invalid CTAP error code: {:x}", packet[0]), + format!("Invalid CTAP error code: {:x}", status_byte), )); }; - let data = if packet.len() > 1 { - Some(Vec::from(&packet[1..])) - } else { + let data = if body.is_empty() { None + } else { + Some(Vec::from(body)) }; Ok(CborResponse { status_code, data }) } diff --git a/libwebauthn/src/proto/ctap2/model/get_assertion.rs b/libwebauthn/src/proto/ctap2/model/get_assertion.rs index dd61a2a..10923f4 100644 --- a/libwebauthn/src/proto/ctap2/model/get_assertion.rs +++ b/libwebauthn/src/proto/ctap2/model/get_assertion.rs @@ -332,8 +332,9 @@ impl Ctap2GetAssertionRequestExtensions { let mut hasher = Sha256::default(); hasher.update(salt1_input); - let salt1_hash = hasher.finalize().to_vec(); - input.salt1.copy_from_slice(&salt1_hash[..32]); + // SHA-256 produces a fixed 32-byte output, which lines up with salt1. + let salt1_hash: [u8; 32] = hasher.finalize().into(); + input.salt1.copy_from_slice(&salt1_hash); // 5.2 If ev.second is present, let salt2 be the value of SHA-256(UTF8Encode("WebAuthn PRF") || 0x00 || ev.second). if let Some(second) = ev.second { @@ -341,10 +342,8 @@ impl Ctap2GetAssertionRequestExtensions { salt2_input.extend(second); let mut hasher = Sha256::default(); hasher.update(salt2_input); - let salt2_hash = hasher.finalize().to_vec(); - let mut salt2 = [0u8; 32]; - salt2.copy_from_slice(&salt2_hash[..32]); - input.salt2 = Some(salt2); + let salt2_hash: [u8; 32] = hasher.finalize().into(); + input.salt2 = Some(salt2_hash); }; Ok(Some(input)) @@ -481,7 +480,7 @@ impl Ctap2GetAssertionResponse { // We always return it, for convenience. let credential_id = self.credential_id.or_else(|| { if request.allow.len() == 1 { - Some(request.allow[0].clone()) + request.allow.first().cloned() } else { None } From 19d51f334d4622cc49e4a4ad4d9f16cae1642895 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 10 May 2026 21:13:19 +0100 Subject: [PATCH 3/5] fix(cable): bounds-check slicing flagged by clippy::indexing_slicing - `crypto::trial_decrypt_advert`: switch to `.get()` for all EID-key and candidate-advert subslices, returning `None` on length mismatch (the up-front length checks make this dead in practice, but the lint requires explicit bounded access). - `crypto::reserved_bits_are_zero`: use `.first().copied()`. - `digit_encode`: rewrite to iterate `chunks(CHUNK_SIZE)` over the input and use `.get()` on the zero-padding lookup; behaviour is preserved (verified by the existing unit test). - `tunnel::CableTunnelMessage::from_slice`: use `split_first` instead of indexing `slice[0]` after a manual length check. - `tunnel::decode_tunnel_server_domain`: use `KNOWN_TUNNEL_DOMAINS.get()`, convert the SHA-256 digest into a fixed `[u8; 32]`, and use `BASE32_CHARS.get()` / `TLDS.get()`. - `tunnel::connect` (initial msg buffer) and the trace log: bound the buffer slice via `.get()`. - `tunnel::send_cbor` padding-length byte: use `last_mut`. --- libwebauthn/src/transport/cable/crypto.rs | 19 +++++--- .../src/transport/cable/digit_encode.rs | 27 ++++++------ libwebauthn/src/transport/cable/tunnel.rs | 44 +++++++++++-------- 3 files changed, 52 insertions(+), 38 deletions(-) diff --git a/libwebauthn/src/transport/cable/crypto.rs b/libwebauthn/src/transport/cable/crypto.rs index ee9f6d7..be9a992 100644 --- a/libwebauthn/src/transport/cable/crypto.rs +++ b/libwebauthn/src/transport/cable/crypto.rs @@ -26,11 +26,13 @@ pub fn derive(secret: &[u8], salt: Option<&[u8]>, purpose: KeyPurpose) -> Result } fn reserved_bits_are_zero(plaintext: &[u8]) -> bool { - plaintext[0] == 0 + plaintext.first().copied() == Some(0) } #[instrument] pub fn trial_decrypt_advert(eid_key: &[u8], candidate_advert: &[u8]) -> Option<[u8; 16]> { + // Both lengths are checked up front so the subsequent slicing is in bounds; + // use `.get(..)` regardless so the clippy::indexing_slicing lint is satisfied. if candidate_advert.len() != 20 { warn!("candidate advert is not 20 bytes"); return None; @@ -41,15 +43,20 @@ pub fn trial_decrypt_advert(eid_key: &[u8], candidate_advert: &[u8]) -> Option<[ return None; } - let expected_tag = hmac_sha256(&eid_key[32..], &candidate_advert[..16]).ok()?; - if expected_tag[..4] != candidate_advert[16..] { - warn!({ expected = ?expected_tag[..4], actual = ?candidate_advert[16..] }, + let mac_key = eid_key.get(32..)?; + let advert_body = candidate_advert.get(..16)?; + let advert_tag = candidate_advert.get(16..)?; + let expected_tag = hmac_sha256(mac_key, advert_body).ok()?; + let expected_tag_truncated = expected_tag.get(..4)?; + if expected_tag_truncated != advert_tag { + warn!({ expected = ?expected_tag_truncated, actual = ?advert_tag }, "candidate advert HMAC tag does not match"); return None; } - let cipher = Aes256::new(GenericArray::from_slice(&eid_key[..32])); - let mut block = Block::clone_from_slice(&candidate_advert[..16]); + let aes_key = eid_key.get(..32)?; + let cipher = Aes256::new(GenericArray::from_slice(aes_key)); + let mut block = Block::clone_from_slice(advert_body); cipher.decrypt_block(&mut block); if !reserved_bits_are_zero(&block) { diff --git a/libwebauthn/src/transport/cable/digit_encode.rs b/libwebauthn/src/transport/cable/digit_encode.rs index 8a40bca..a7f96bb 100644 --- a/libwebauthn/src/transport/cable/digit_encode.rs +++ b/libwebauthn/src/transport/cable/digit_encode.rs @@ -8,23 +8,22 @@ const PARTIAL_CHUNK_DIGITS: usize = 0x0fda8530; pub fn digit_encode(input: &[u8]) -> String { let mut output = String::new(); - let mut input = input; - while input.len() >= CHUNK_SIZE { + for chunk_slice in input.chunks(CHUNK_SIZE) { let mut chunk = [0u8; 8]; - chunk[..CHUNK_SIZE].copy_from_slice(&input[..CHUNK_SIZE]); + let (head, _) = chunk.split_at_mut(chunk_slice.len()); + head.copy_from_slice(chunk_slice); let v = u64::from_le_bytes(chunk); let v = v.to_string(); - output.push_str(&ZEROS[..CHUNK_DIGITS - v.len()]); - output.push_str(&v); - input = &input[CHUNK_SIZE..]; - } - if !input.is_empty() { - let digits = 0x0F & (PARTIAL_CHUNK_DIGITS >> (4 * input.len())); - let mut chunk = [0u8; 8]; - chunk[..input.len()].copy_from_slice(input); - let v = u64::from_le_bytes(chunk); - let v = v.to_string(); - output.push_str(&ZEROS[..digits - v.len()]); + let digits = if chunk_slice.len() == CHUNK_SIZE { + CHUNK_DIGITS + } else { + 0x0F & (PARTIAL_CHUNK_DIGITS >> (4 * chunk_slice.len())) + }; + // ZEROS is 17 chars (CHUNK_DIGITS); slice within bounds. + let pad_len = digits.saturating_sub(v.len()); + if let Some(pad) = ZEROS.get(..pad_len) { + output.push_str(pad); + } output.push_str(&v); } output diff --git a/libwebauthn/src/transport/cable/tunnel.rs b/libwebauthn/src/transport/cable/tunnel.rs index 1a4f929..35afae0 100644 --- a/libwebauthn/src/transport/cable/tunnel.rs +++ b/libwebauthn/src/transport/cable/tunnel.rs @@ -61,11 +61,14 @@ impl CableTunnelMessage { } } pub fn from_slice(slice: &[u8]) -> Result { - if slice.len() < 2 { + let (type_byte, payload) = slice + .split_first() + .ok_or(Error::Transport(TransportError::InvalidFraming))?; + if payload.is_empty() { return Err(Error::Transport(TransportError::InvalidFraming)); } - let message_type = match slice[0] { + let message_type = match *type_byte { 0 => CableTunnelMessageType::Shutdown, 1 => CableTunnelMessageType::Ctap, 2 => CableTunnelMessageType::Update, @@ -76,7 +79,7 @@ impl CableTunnelMessage { Ok(Self { message_type, - payload: ByteBuf::from(slice[1..].to_vec()), + payload: ByteBuf::from(payload.to_vec()), }) } @@ -131,10 +134,9 @@ enum CableTunnelMessageType { pub fn decode_tunnel_server_domain(encoded: u16) -> Option { if encoded < 256 { - if encoded as usize >= KNOWN_TUNNEL_DOMAINS.len() { - return None; - } - return Some(KNOWN_TUNNEL_DOMAINS[encoded as usize].to_string()); + return KNOWN_TUNNEL_DOMAINS + .get(encoded as usize) + .map(|s| (*s).to_string()); } let mut sha_input = SHA_INPUT.to_vec(); @@ -143,21 +145,22 @@ pub fn decode_tunnel_server_domain(encoded: u16) -> Option { sha_input.push(0); let mut hasher = Sha256::default(); hasher.update(&sha_input); - let digest = hasher.finalize(); - - let mut v = u64::from_le_bytes([ - digest[0], digest[1], digest[2], digest[3], digest[4], digest[5], digest[6], digest[7], - ]); - let tld_index = v & 3; + // SHA-256 produces 32 bytes, so the first 8 bytes are always present. + let digest: [u8; 32] = hasher.finalize().into(); + let mut digest_head = [0u8; 8]; + digest_head.copy_from_slice(digest.get(..8)?); + let mut v = u64::from_le_bytes(digest_head); + let tld_index = (v & 3) as usize; v >>= 2; let mut ret = String::from("cable."); while v != 0 { - ret.push(BASE32_CHARS[(v & 31) as usize] as char); + let ch = *BASE32_CHARS.get((v & 31) as usize)?; + ret.push(ch as char); v >>= 5; } - ret.push_str(TLDS[tld_index as usize]); + ret.push_str(TLDS.get(tld_index)?); Some(ret) } @@ -310,7 +313,10 @@ pub(crate) async fn do_handshake( } }; - let initial_msg: Vec = initial_msg_buffer[..initial_msg_len].into(); + let initial_msg: Vec = initial_msg_buffer + .get(..initial_msg_len) + .map(<[u8]>::to_vec) + .ok_or(TransportError::ConnectionFailed)?; trace!( { handshake = ?initial_msg }, "Sending initial handshake message" @@ -366,7 +372,7 @@ pub(crate) async fn do_handshake( }; debug!( - { handshake = ?payload[..payload_len] }, + { handshake = ?payload.get(..payload_len) }, "Received handshake response" ); @@ -466,7 +472,9 @@ async fn connection_send( let mut padded_cbor_request = cbor_request.clone(); padded_cbor_request.resize(padded_len, 0u8); - padded_cbor_request[padded_len - 1] = (extra_bytes - 1) as u8; + if let Some(last) = padded_cbor_request.last_mut() { + *last = (extra_bytes - 1) as u8; + } let frame = CableTunnelMessage::new(CableTunnelMessageType::Ctap, &padded_cbor_request); let frame_serialized = frame.to_vec(); From 2846e2fb6ff235b094f814b8803882ab8f290f51 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 10 May 2026 21:13:25 +0100 Subject: [PATCH 4/5] fix(transport): bounds-check slicing in BLE and HID framing In `transport::ble::framing` and `transport::hid::framing`, rewrite `frame()` / `message()`, `expected_bytes()`, and length helpers to use `split_first`, `.get()`, and `saturating_sub` instead of direct indexing. The behaviour is preserved (verified by the existing unit tests). In `transport::hid::channel::open` the INIT nonce comparison now uses `response.payload.get(..INIT_NONCE_LEN)` rather than the previously panic-prone slice index; the explicit length check on the line above makes this safe in practice but the lint requires bounded access. --- libwebauthn/src/transport/ble/framing.rs | 44 +++++++++++++++--------- libwebauthn/src/transport/hid/channel.rs | 6 +++- libwebauthn/src/transport/hid/framing.rs | 35 +++++++++++-------- 3 files changed, 53 insertions(+), 32 deletions(-) diff --git a/libwebauthn/src/transport/ble/framing.rs b/libwebauthn/src/transport/ble/framing.rs index a8b0326..9f92bdb 100644 --- a/libwebauthn/src/transport/ble/framing.rs +++ b/libwebauthn/src/transport/ble/framing.rs @@ -126,16 +126,25 @@ impl BleFrameParser { )); } - let cmd = self.fragments[0][0].try_into().or(Err(IOError::new( + let (initial, continuations) = self + .fragments + .split_first() + .ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Frame has no fragments"))?; + let cmd_byte = *initial + .first() + .ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Initial fragment is empty"))?; + let cmd = cmd_byte.try_into().or(Err(IOError::new( IOErrorKind::InvalidData, - format!("Invalid BLE frame command: {:x}", self.fragments[0][0]), + format!("Invalid BLE frame command: {:x}", cmd_byte), )))?; let mut data = vec![]; - data.extend(&self.fragments[0][INITIAL_FRAGMENT_HEADER_LENGTH..self.fragments[0].len()]); - for cont_fragment in &self.fragments[1..self.fragments.len()] { - data.extend_from_slice( - &cont_fragment[CONT_FRAGMENT_HEADER_LENGTH..cont_fragment.len()], - ); + if let Some(initial_data) = initial.get(INITIAL_FRAGMENT_HEADER_LENGTH..) { + data.extend(initial_data); + } + for cont_fragment in continuations { + if let Some(cont_data) = cont_fragment.get(CONT_FRAGMENT_HEADER_LENGTH..) { + data.extend_from_slice(cont_data); + } } Ok(BleFrame::new(cmd, &data)) @@ -154,22 +163,23 @@ impl BleFrameParser { } fn expected_bytes(&self) -> Option { - if self.fragments.is_empty() { - return None; - } - - let mut cursor = IOCursor::new(vec![self.fragments[0][1], self.fragments[0][2]]); + let initial = self.fragments.first()?; + let b1 = *initial.get(1)?; + let b2 = *initial.get(2)?; + let mut cursor = IOCursor::new(vec![b1, b2]); Some(cursor.read_u16::().ok()? as usize) } fn data_len(&self) -> usize { - if self.fragments.is_empty() { + let Some((initial, continuations)) = self.fragments.split_first() else { return 0; - } + }; - let mut data_len = self.fragments[0].len() - INITIAL_FRAGMENT_HEADER_LENGTH; - for cont_fragment in &self.fragments[1..self.fragments.len()] { - data_len += cont_fragment.len() - CONT_FRAGMENT_HEADER_LENGTH; + let mut data_len = initial.len().saturating_sub(INITIAL_FRAGMENT_HEADER_LENGTH); + for cont_fragment in continuations { + data_len += cont_fragment + .len() + .saturating_sub(CONT_FRAGMENT_HEADER_LENGTH); } data_len } diff --git a/libwebauthn/src/transport/hid/channel.rs b/libwebauthn/src/transport/hid/channel.rs index e56f7aa..cdcb300 100644 --- a/libwebauthn/src/transport/hid/channel.rs +++ b/libwebauthn/src/transport/hid/channel.rs @@ -189,7 +189,11 @@ impl<'d> HidChannel<'d> { return Err(Error::Transport(TransportError::InvalidEndpoint)); } - if response.payload[0..INIT_NONCE_LEN] != nonce[0..INIT_NONCE_LEN] { + let payload_nonce = response + .payload + .get(..INIT_NONCE_LEN) + .ok_or(Error::Transport(TransportError::InvalidEndpoint))?; + if payload_nonce != nonce.as_slice() { warn!("INIT nonce mismatch. Terminating."); return Err(Error::Transport(TransportError::InvalidEndpoint)); } diff --git a/libwebauthn/src/transport/hid/framing.rs b/libwebauthn/src/transport/hid/framing.rs index 9785100..69da48a 100644 --- a/libwebauthn/src/transport/hid/framing.rs +++ b/libwebauthn/src/transport/hid/framing.rs @@ -147,22 +147,21 @@ impl HidMessageParser { } fn expected_bytes(&self) -> Option { - if self.packets.is_empty() { - return None; - } - - let mut cursor = IOCursor::new(vec![self.packets[0][5], self.packets[0][6]]); + let initial = self.packets.first()?; + let b5 = *initial.get(5)?; + let b6 = *initial.get(6)?; + let mut cursor = IOCursor::new(vec![b5, b6]); Some(cursor.read_u16::().ok()? as usize) } fn payload_len(&self) -> usize { - if self.packets.is_empty() { + let Some((initial, continuations)) = self.packets.split_first() else { return 0; - } + }; - let mut payload_len = self.packets[0].len() - PACKET_INITIAL_HEADER_SIZE; - for cont_packet in &self.packets[1..self.packets.len()] { - payload_len += cont_packet.len() - PACKET_CONT_HEADER_SIZE; + let mut payload_len = initial.len().saturating_sub(PACKET_INITIAL_HEADER_SIZE); + for cont_packet in continuations { + payload_len += cont_packet.len().saturating_sub(PACKET_CONT_HEADER_SIZE); } payload_len } @@ -175,7 +174,11 @@ impl HidMessageParser { )); } - let mut cursor = IOCursor::new(&self.packets[0]); + let (initial, continuations) = self + .packets + .split_first() + .ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Message has no packets"))?; + let mut cursor = IOCursor::new(initial); let cid = cursor.read_u32::()?; let cmd = cursor.read_u8()? ^ PACKET_INITIAL_CMD_MASK; let Ok(cmd) = cmd.try_into() else { @@ -188,9 +191,13 @@ impl HidMessageParser { let expected_size = cursor.read_u16::()?; let mut payload = vec![]; - payload.extend(&self.packets[0][PACKET_INITIAL_HEADER_SIZE..]); - for cont_packet in &self.packets[1..] { - payload.extend_from_slice(&cont_packet[PACKET_CONT_HEADER_SIZE..]); + if let Some(initial_payload) = initial.get(PACKET_INITIAL_HEADER_SIZE..) { + payload.extend(initial_payload); + } + for cont_packet in continuations { + if let Some(cont_payload) = cont_packet.get(PACKET_CONT_HEADER_SIZE..) { + payload.extend_from_slice(cont_payload); + } } payload.truncate(expected_size as usize); From 782afa7d4f7f21bee7ce984e1e3b69cc0b783065 Mon Sep 17 00:00:00 2001 From: Alfie Fresta Date: Sun, 10 May 2026 21:39:34 +0100 Subject: [PATCH 5/5] fix(nfc): bounds-check slicing flagged by clippy::indexing_slicing The NFC backend (gated behind the `nfc`/`pcsc`/`libnfc` features) was not exercised by the default `cargo build`, so the lint enabled in the previous commits did not surface there. CI's `cargo clippy --all-targets --all-features` flags 5 sites: - `channel::NfcChannel::handle`: replace `&buf[..len]` (returned by `handle_in_ctx` into a fixed 1024-byte buffer) with `buf.get(..len)` and surface `HandleError::NotEnoughBuffer` if a backend overruns. - `channel::NfcChannel::cbor_send`: replace the `&rest[..250]` / `&rest[250..]` chunking with `rest.split_at(250)`; the `rest.len() > 250` loop predicate keeps this panic-safe. - `libnfc::Channel::connect_to_target`: replace `&modulations[modulations.len() - 1]` with `.last()`, returning `TransportUnavailable` if the device reports no supported baud rates. --- libwebauthn/src/transport/nfc/channel.rs | 10 ++++++---- libwebauthn/src/transport/nfc/libnfc/mod.rs | 4 +++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/libwebauthn/src/transport/nfc/channel.rs b/libwebauthn/src/transport/nfc/channel.rs index c6b15fb..3e66649 100644 --- a/libwebauthn/src/transport/nfc/channel.rs +++ b/libwebauthn/src/transport/nfc/channel.rs @@ -196,7 +196,8 @@ where let mut rapdu = Vec::new(); let len = self.handle_in_ctx(ctx, &command_buf, &mut buf)?; - let mut resp = Response::from(&buf[..len]); + let resp_bytes = buf.get(..len).ok_or(HandleError::NotEnoughBuffer(len))?; + let mut resp = Response::from(resp_bytes); let (mut sw1, mut sw2) = resp.trailer; rapdu.extend_from_slice(resp.payload); @@ -205,7 +206,8 @@ where let get_response_cmd = command_get_response(0x00, 0x00, sw2); let get_response_buf = Vec::from(get_response_cmd); let len = self.handle_in_ctx(ctx, &get_response_buf, &mut buf)?; - resp = Response::from(&buf[..len]); + let resp_bytes = buf.get(..len).ok_or(HandleError::NotEnoughBuffer(len))?; + resp = Response::from(resp_bytes); (sw1, sw2) = resp.trailer; rapdu.extend_from_slice(resp.payload); } @@ -272,8 +274,8 @@ where let mut rest: &[u8] = data; while rest.len() > 250 { - let to_send = &rest[..250]; - rest = &rest[250..]; + let (to_send, remaining) = rest.split_at(250); + rest = remaining; let ctap_msg = command_ctap_msg(true, to_send); let resp = self.handle(self.ctx, ctap_msg)?; trace!("cbor_send has_more {:?} {:?}", to_send, resp); diff --git a/libwebauthn/src/transport/nfc/libnfc/mod.rs b/libwebauthn/src/transport/nfc/libnfc/mod.rs index f77d718..d47d0ea 100644 --- a/libwebauthn/src/transport/nfc/libnfc/mod.rs +++ b/libwebauthn/src/transport/nfc/libnfc/mod.rs @@ -150,7 +150,9 @@ impl Channel { baud_rate: *baud_rate, }) .collect::>(); - let modulation = &modulations[modulations.len() - 1]; + let modulation = modulations + .last() + .ok_or(Error::Transport(TransportError::TransportUnavailable))?; let is_one_rate = modulations.len() == 1; for i in 0..2 { if i > 0 {