Skip to content

Commit f78e4c1

Browse files
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.
1 parent 88c1612 commit f78e4c1

6 files changed

Lines changed: 59 additions & 30 deletions

File tree

libwebauthn/src/fido.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,11 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for AuthenticatorData<T> {
249249
};
250250

251251
// Check if we have trailing data
252-
if !&data[cursor.position() as usize..].is_empty() {
252+
let pos = cursor.position() as usize;
253+
let trailing = data.get(pos..).ok_or_else(|| {
254+
DesError::custom("cursor advanced past end of authenticator data")
255+
})?;
256+
if !trailing.is_empty() {
253257
return Err(DesError::invalid_length(data.len(), &"trailing data"));
254258
}
255259

libwebauthn/src/pin.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,14 @@ impl PinUvAuthProtocol for PinUvAuthProtocolOne {
264264
fn authenticate(&self, key: &[u8], message: &[u8]) -> Result<Vec<u8>, Error> {
265265
// Return the first 16 bytes of the result of computing HMAC-SHA-256 with the given key and message.
266266
let hmac = hmac_sha256(key, message)?;
267-
Ok(Vec::from(&hmac[..16]))
267+
// HMAC-SHA-256 produces 32 bytes, so this slice is always valid.
268+
let truncated = hmac.get(..16).ok_or_else(|| {
269+
error!(len = hmac.len(), "HMAC output shorter than 16 bytes");
270+
Error::Platform(PlatformError::CryptoError(
271+
"HMAC output shorter than 16 bytes".into(),
272+
))
273+
})?;
274+
Ok(Vec::from(truncated))
268275
}
269276

270277
#[instrument(skip_all)]
@@ -440,8 +447,9 @@ impl PinUvAuthProtocol for PinUvAuthProtocolTwo {
440447
pub fn pin_hash(pin: &[u8]) -> Vec<u8> {
441448
let mut hasher = Sha256::default();
442449
hasher.update(pin);
443-
let hashed = hasher.finalize().to_vec();
444-
Vec::from(&hashed[..16])
450+
let hashed = hasher.finalize();
451+
// SHA-256 output is fixed at 32 bytes; keep only the first 16 per the spec.
452+
hashed.into_iter().take(16).collect()
445453
}
446454

447455
pub fn hmac_sha256(key: &[u8], message: &[u8]) -> Result<Vec<u8>, Error> {

libwebauthn/src/proto/ctap1/apdu/response.rs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,26 @@ impl ApduResponse {
4545
impl TryFrom<&Vec<u8>> for ApduResponse {
4646
type Error = IOError;
4747
fn try_from(packet: &Vec<u8>) -> Result<Self, Self::Error> {
48-
if packet.len() < 2 {
49-
return Err(IOError::new(
48+
let split_at = packet.len().checked_sub(2).ok_or_else(|| {
49+
IOError::new(
5050
IOErrorKind::InvalidData,
5151
"Apdu response packets must contain at least 2 bytes.",
52-
));
53-
}
52+
)
53+
})?;
54+
let (body, status) = packet.split_at(split_at);
55+
// `status` is guaranteed to have exactly 2 elements by construction above.
56+
let sw1 = *status
57+
.first()
58+
.ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Missing APDU status byte 1"))?;
59+
let sw2 = *status
60+
.get(1)
61+
.ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Missing APDU status byte 2"))?;
5462

55-
let data = if packet.len() > 2 {
56-
Some(Vec::from(&packet[0..packet.len() - 2]))
57-
} else {
63+
let data = if body.is_empty() {
5864
None
65+
} else {
66+
Some(Vec::from(body))
5967
};
60-
let (sw1, sw2) = (packet[packet.len() - 2], packet[packet.len() - 1]);
6168

6269
Ok(Self { data, sw1, sw2 })
6370
}

libwebauthn/src/proto/ctap1/model.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,18 @@ impl TryFrom<ApduResponse> for Ctap1RegisterResponse {
142142
"Failed to parse X509 attestation data",
143143
)))?;
144144
let signature = Vec::from(signature);
145-
let attestation = Vec::from(&remaining[0..remaining.len() - signature.len()]);
145+
// `signature` is a suffix of `remaining` returned by `X509Certificate::from_der`,
146+
// so the split index is always within bounds in practice; bound it explicitly.
147+
let split_at = remaining
148+
.len()
149+
.checked_sub(signature.len())
150+
.ok_or_else(|| {
151+
IOError::new(
152+
IOErrorKind::InvalidData,
153+
"Signature longer than remaining U2F register response data",
154+
)
155+
})?;
156+
let attestation = Vec::from(remaining.get(..split_at).unwrap_or(&[]));
146157

147158
Ok(Ctap1RegisterResponse {
148159
version: Ctap1Version::U2fV2,

libwebauthn/src/proto/ctap2/cbor/response.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,25 @@ impl CborResponse {
2525
impl TryFrom<&Vec<u8>> for CborResponse {
2626
type Error = IOError;
2727
fn try_from(packet: &Vec<u8>) -> Result<Self, Self::Error> {
28-
if packet.is_empty() {
29-
return Err(IOError::new(
28+
let (status_byte, body) = packet.split_first().ok_or_else(|| {
29+
IOError::new(
3030
IOErrorKind::InvalidData,
3131
"Cbor response packets must contain at least 1 byte.",
32-
));
33-
}
32+
)
33+
})?;
3434

35-
let Ok(status_code) = packet[0].try_into() else {
36-
error!({ code = ?packet[0] }, "Invalid CTAP error code");
35+
let Ok(status_code) = (*status_byte).try_into() else {
36+
error!({ code = ?*status_byte }, "Invalid CTAP error code");
3737
return Err(IOError::new(
3838
IOErrorKind::InvalidData,
39-
format!("Invalid CTAP error code: {:x}", packet[0]),
39+
format!("Invalid CTAP error code: {:x}", status_byte),
4040
));
4141
};
4242

43-
let data = if packet.len() > 1 {
44-
Some(Vec::from(&packet[1..]))
45-
} else {
43+
let data = if body.is_empty() {
4644
None
45+
} else {
46+
Some(Vec::from(body))
4747
};
4848
Ok(CborResponse { status_code, data })
4949
}

libwebauthn/src/proto/ctap2/model/get_assertion.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -332,19 +332,18 @@ impl Ctap2GetAssertionRequestExtensions {
332332

333333
let mut hasher = Sha256::default();
334334
hasher.update(salt1_input);
335-
let salt1_hash = hasher.finalize().to_vec();
336-
input.salt1.copy_from_slice(&salt1_hash[..32]);
335+
// SHA-256 produces a fixed 32-byte output, which lines up with salt1.
336+
let salt1_hash: [u8; 32] = hasher.finalize().into();
337+
input.salt1.copy_from_slice(&salt1_hash);
337338

338339
// 5.2 If ev.second is present, let salt2 be the value of SHA-256(UTF8Encode("WebAuthn PRF") || 0x00 || ev.second).
339340
if let Some(second) = ev.second {
340341
let mut salt2_input = prefix.clone();
341342
salt2_input.extend(second);
342343
let mut hasher = Sha256::default();
343344
hasher.update(salt2_input);
344-
let salt2_hash = hasher.finalize().to_vec();
345-
let mut salt2 = [0u8; 32];
346-
salt2.copy_from_slice(&salt2_hash[..32]);
347-
input.salt2 = Some(salt2);
345+
let salt2_hash: [u8; 32] = hasher.finalize().into();
346+
input.salt2 = Some(salt2_hash);
348347
};
349348

350349
Ok(Some(input))
@@ -481,7 +480,7 @@ impl Ctap2GetAssertionResponse {
481480
// We always return it, for convenience.
482481
let credential_id = self.credential_id.or_else(|| {
483482
if request.allow.len() == 1 {
484-
Some(request.allow[0].clone())
483+
request.allow.first().cloned()
485484
} else {
486485
None
487486
}

0 commit comments

Comments
 (0)