Skip to content

Commit 9f888c0

Browse files
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`.
1 parent 061e757 commit 9f888c0

3 files changed

Lines changed: 52 additions & 38 deletions

File tree

libwebauthn/src/transport/cable/crypto.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,13 @@ pub fn derive(secret: &[u8], salt: Option<&[u8]>, purpose: KeyPurpose) -> Result
2626
}
2727

2828
fn reserved_bits_are_zero(plaintext: &[u8]) -> bool {
29-
plaintext[0] == 0
29+
plaintext.first().copied() == Some(0)
3030
}
3131

3232
#[instrument]
3333
pub fn trial_decrypt_advert(eid_key: &[u8], candidate_advert: &[u8]) -> Option<[u8; 16]> {
34+
// Both lengths are checked up front so the subsequent slicing is in bounds;
35+
// use `.get(..)` regardless so the clippy::indexing_slicing lint is satisfied.
3436
if candidate_advert.len() != 20 {
3537
warn!("candidate advert is not 20 bytes");
3638
return None;
@@ -41,15 +43,20 @@ pub fn trial_decrypt_advert(eid_key: &[u8], candidate_advert: &[u8]) -> Option<[
4143
return None;
4244
}
4345

44-
let expected_tag = hmac_sha256(&eid_key[32..], &candidate_advert[..16]).ok()?;
45-
if expected_tag[..4] != candidate_advert[16..] {
46-
warn!({ expected = ?expected_tag[..4], actual = ?candidate_advert[16..] },
46+
let mac_key = eid_key.get(32..)?;
47+
let advert_body = candidate_advert.get(..16)?;
48+
let advert_tag = candidate_advert.get(16..)?;
49+
let expected_tag = hmac_sha256(mac_key, advert_body).ok()?;
50+
let expected_tag_truncated = expected_tag.get(..4)?;
51+
if expected_tag_truncated != advert_tag {
52+
warn!({ expected = ?expected_tag_truncated, actual = ?advert_tag },
4753
"candidate advert HMAC tag does not match");
4854
return None;
4955
}
5056

51-
let cipher = Aes256::new(GenericArray::from_slice(&eid_key[..32]));
52-
let mut block = Block::clone_from_slice(&candidate_advert[..16]);
57+
let aes_key = eid_key.get(..32)?;
58+
let cipher = Aes256::new(GenericArray::from_slice(aes_key));
59+
let mut block = Block::clone_from_slice(advert_body);
5360
cipher.decrypt_block(&mut block);
5461

5562
if !reserved_bits_are_zero(&block) {

libwebauthn/src/transport/cable/digit_encode.rs

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,22 @@ const PARTIAL_CHUNK_DIGITS: usize = 0x0fda8530;
88

99
pub fn digit_encode(input: &[u8]) -> String {
1010
let mut output = String::new();
11-
let mut input = input;
12-
while input.len() >= CHUNK_SIZE {
11+
for chunk_slice in input.chunks(CHUNK_SIZE) {
1312
let mut chunk = [0u8; 8];
14-
chunk[..CHUNK_SIZE].copy_from_slice(&input[..CHUNK_SIZE]);
13+
let (head, _) = chunk.split_at_mut(chunk_slice.len());
14+
head.copy_from_slice(chunk_slice);
1515
let v = u64::from_le_bytes(chunk);
1616
let v = v.to_string();
17-
output.push_str(&ZEROS[..CHUNK_DIGITS - v.len()]);
18-
output.push_str(&v);
19-
input = &input[CHUNK_SIZE..];
20-
}
21-
if !input.is_empty() {
22-
let digits = 0x0F & (PARTIAL_CHUNK_DIGITS >> (4 * input.len()));
23-
let mut chunk = [0u8; 8];
24-
chunk[..input.len()].copy_from_slice(input);
25-
let v = u64::from_le_bytes(chunk);
26-
let v = v.to_string();
27-
output.push_str(&ZEROS[..digits - v.len()]);
17+
let digits = if chunk_slice.len() == CHUNK_SIZE {
18+
CHUNK_DIGITS
19+
} else {
20+
0x0F & (PARTIAL_CHUNK_DIGITS >> (4 * chunk_slice.len()))
21+
};
22+
// ZEROS is 17 chars (CHUNK_DIGITS); slice within bounds.
23+
let pad_len = digits.saturating_sub(v.len());
24+
if let Some(pad) = ZEROS.get(..pad_len) {
25+
output.push_str(pad);
26+
}
2827
output.push_str(&v);
2928
}
3029
output

libwebauthn/src/transport/cable/tunnel.rs

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,14 @@ impl CableTunnelMessage {
6161
}
6262
}
6363
pub fn from_slice(slice: &[u8]) -> Result<Self, Error> {
64-
if slice.len() < 2 {
64+
let (type_byte, payload) = slice
65+
.split_first()
66+
.ok_or(Error::Transport(TransportError::InvalidFraming))?;
67+
if payload.is_empty() {
6568
return Err(Error::Transport(TransportError::InvalidFraming));
6669
}
6770

68-
let message_type = match slice[0] {
71+
let message_type = match *type_byte {
6972
0 => CableTunnelMessageType::Shutdown,
7073
1 => CableTunnelMessageType::Ctap,
7174
2 => CableTunnelMessageType::Update,
@@ -76,7 +79,7 @@ impl CableTunnelMessage {
7679

7780
Ok(Self {
7881
message_type,
79-
payload: ByteBuf::from(slice[1..].to_vec()),
82+
payload: ByteBuf::from(payload.to_vec()),
8083
})
8184
}
8285

@@ -131,10 +134,9 @@ enum CableTunnelMessageType {
131134

132135
pub fn decode_tunnel_server_domain(encoded: u16) -> Option<String> {
133136
if encoded < 256 {
134-
if encoded as usize >= KNOWN_TUNNEL_DOMAINS.len() {
135-
return None;
136-
}
137-
return Some(KNOWN_TUNNEL_DOMAINS[encoded as usize].to_string());
137+
return KNOWN_TUNNEL_DOMAINS
138+
.get(encoded as usize)
139+
.map(|s| (*s).to_string());
138140
}
139141

140142
let mut sha_input = SHA_INPUT.to_vec();
@@ -143,21 +145,22 @@ pub fn decode_tunnel_server_domain(encoded: u16) -> Option<String> {
143145
sha_input.push(0);
144146
let mut hasher = Sha256::default();
145147
hasher.update(&sha_input);
146-
let digest = hasher.finalize();
147-
148-
let mut v = u64::from_le_bytes([
149-
digest[0], digest[1], digest[2], digest[3], digest[4], digest[5], digest[6], digest[7],
150-
]);
151-
let tld_index = v & 3;
148+
// SHA-256 produces 32 bytes, so the first 8 bytes are always present.
149+
let digest: [u8; 32] = hasher.finalize().into();
150+
let mut digest_head = [0u8; 8];
151+
digest_head.copy_from_slice(digest.get(..8)?);
152+
let mut v = u64::from_le_bytes(digest_head);
153+
let tld_index = (v & 3) as usize;
152154
v >>= 2;
153155

154156
let mut ret = String::from("cable.");
155157
while v != 0 {
156-
ret.push(BASE32_CHARS[(v & 31) as usize] as char);
158+
let ch = *BASE32_CHARS.get((v & 31) as usize)?;
159+
ret.push(ch as char);
157160
v >>= 5;
158161
}
159162

160-
ret.push_str(TLDS[tld_index as usize]);
163+
ret.push_str(TLDS.get(tld_index)?);
161164
Some(ret)
162165
}
163166

@@ -310,7 +313,10 @@ pub(crate) async fn do_handshake(
310313
}
311314
};
312315

313-
let initial_msg: Vec<u8> = initial_msg_buffer[..initial_msg_len].into();
316+
let initial_msg: Vec<u8> = initial_msg_buffer
317+
.get(..initial_msg_len)
318+
.map(<[u8]>::to_vec)
319+
.ok_or(TransportError::ConnectionFailed)?;
314320
trace!(
315321
{ handshake = ?initial_msg },
316322
"Sending initial handshake message"
@@ -366,7 +372,7 @@ pub(crate) async fn do_handshake(
366372
};
367373

368374
debug!(
369-
{ handshake = ?payload[..payload_len] },
375+
{ handshake = ?payload.get(..payload_len) },
370376
"Received handshake response"
371377
);
372378

@@ -466,7 +472,9 @@ async fn connection_send(
466472

467473
let mut padded_cbor_request = cbor_request.clone();
468474
padded_cbor_request.resize(padded_len, 0u8);
469-
padded_cbor_request[padded_len - 1] = (extra_bytes - 1) as u8;
475+
if let Some(last) = padded_cbor_request.last_mut() {
476+
*last = (extra_bytes - 1) as u8;
477+
}
470478

471479
let frame = CableTunnelMessage::new(CableTunnelMessageType::Ctap, &padded_cbor_request);
472480
let frame_serialized = frame.to_vec();

0 commit comments

Comments
 (0)