Skip to content

Commit e84a8db

Browse files
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.
1 parent e0bb25f commit e84a8db

3 files changed

Lines changed: 53 additions & 32 deletions

File tree

libwebauthn/src/transport/ble/framing.rs

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -126,16 +126,25 @@ impl BleFrameParser {
126126
));
127127
}
128128

129-
let cmd = self.fragments[0][0].try_into().or(Err(IOError::new(
129+
let (initial, continuations) = self
130+
.fragments
131+
.split_first()
132+
.ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Frame has no fragments"))?;
133+
let cmd_byte = *initial
134+
.first()
135+
.ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Initial fragment is empty"))?;
136+
let cmd = cmd_byte.try_into().or(Err(IOError::new(
130137
IOErrorKind::InvalidData,
131-
format!("Invalid BLE frame command: {:x}", self.fragments[0][0]),
138+
format!("Invalid BLE frame command: {:x}", cmd_byte),
132139
)))?;
133140
let mut data = vec![];
134-
data.extend(&self.fragments[0][INITIAL_FRAGMENT_HEADER_LENGTH..self.fragments[0].len()]);
135-
for cont_fragment in &self.fragments[1..self.fragments.len()] {
136-
data.extend_from_slice(
137-
&cont_fragment[CONT_FRAGMENT_HEADER_LENGTH..cont_fragment.len()],
138-
);
141+
if let Some(initial_data) = initial.get(INITIAL_FRAGMENT_HEADER_LENGTH..) {
142+
data.extend(initial_data);
143+
}
144+
for cont_fragment in continuations {
145+
if let Some(cont_data) = cont_fragment.get(CONT_FRAGMENT_HEADER_LENGTH..) {
146+
data.extend_from_slice(cont_data);
147+
}
139148
}
140149

141150
Ok(BleFrame::new(cmd, &data))
@@ -154,22 +163,23 @@ impl BleFrameParser {
154163
}
155164

156165
fn expected_bytes(&self) -> Option<usize> {
157-
if self.fragments.is_empty() {
158-
return None;
159-
}
160-
161-
let mut cursor = IOCursor::new(vec![self.fragments[0][1], self.fragments[0][2]]);
166+
let initial = self.fragments.first()?;
167+
let b1 = *initial.get(1)?;
168+
let b2 = *initial.get(2)?;
169+
let mut cursor = IOCursor::new(vec![b1, b2]);
162170
Some(cursor.read_u16::<BigEndian>().ok()? as usize)
163171
}
164172

165173
fn data_len(&self) -> usize {
166-
if self.fragments.is_empty() {
174+
let Some((initial, continuations)) = self.fragments.split_first() else {
167175
return 0;
168-
}
176+
};
169177

170-
let mut data_len = self.fragments[0].len() - INITIAL_FRAGMENT_HEADER_LENGTH;
171-
for cont_fragment in &self.fragments[1..self.fragments.len()] {
172-
data_len += cont_fragment.len() - CONT_FRAGMENT_HEADER_LENGTH;
178+
let mut data_len = initial.len().saturating_sub(INITIAL_FRAGMENT_HEADER_LENGTH);
179+
for cont_fragment in continuations {
180+
data_len += cont_fragment
181+
.len()
182+
.saturating_sub(CONT_FRAGMENT_HEADER_LENGTH);
173183
}
174184
data_len
175185
}

libwebauthn/src/transport/hid/channel.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,11 @@ impl<'d> HidChannel<'d> {
189189
return Err(Error::Transport(TransportError::InvalidEndpoint));
190190
}
191191

192-
if response.payload[0..INIT_NONCE_LEN] != nonce[0..INIT_NONCE_LEN] {
192+
let payload_nonce = response
193+
.payload
194+
.get(..INIT_NONCE_LEN)
195+
.ok_or(Error::Transport(TransportError::InvalidEndpoint))?;
196+
if payload_nonce != nonce.as_slice() {
193197
warn!("INIT nonce mismatch. Terminating.");
194198
return Err(Error::Transport(TransportError::InvalidEndpoint));
195199
}

libwebauthn/src/transport/hid/framing.rs

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -147,22 +147,21 @@ impl HidMessageParser {
147147
}
148148

149149
fn expected_bytes(&self) -> Option<usize> {
150-
if self.packets.is_empty() {
151-
return None;
152-
}
153-
154-
let mut cursor = IOCursor::new(vec![self.packets[0][5], self.packets[0][6]]);
150+
let initial = self.packets.first()?;
151+
let b5 = *initial.get(5)?;
152+
let b6 = *initial.get(6)?;
153+
let mut cursor = IOCursor::new(vec![b5, b6]);
155154
Some(cursor.read_u16::<BigEndian>().ok()? as usize)
156155
}
157156

158157
fn payload_len(&self) -> usize {
159-
if self.packets.is_empty() {
158+
let Some((initial, continuations)) = self.packets.split_first() else {
160159
return 0;
161-
}
160+
};
162161

163-
let mut payload_len = self.packets[0].len() - PACKET_INITIAL_HEADER_SIZE;
164-
for cont_packet in &self.packets[1..self.packets.len()] {
165-
payload_len += cont_packet.len() - PACKET_CONT_HEADER_SIZE;
162+
let mut payload_len = initial.len().saturating_sub(PACKET_INITIAL_HEADER_SIZE);
163+
for cont_packet in continuations {
164+
payload_len += cont_packet.len().saturating_sub(PACKET_CONT_HEADER_SIZE);
166165
}
167166
payload_len
168167
}
@@ -175,7 +174,11 @@ impl HidMessageParser {
175174
));
176175
}
177176

178-
let mut cursor = IOCursor::new(&self.packets[0]);
177+
let (initial, continuations) = self
178+
.packets
179+
.split_first()
180+
.ok_or_else(|| IOError::new(IOErrorKind::InvalidData, "Message has no packets"))?;
181+
let mut cursor = IOCursor::new(initial);
179182
let cid = cursor.read_u32::<BigEndian>()?;
180183
let cmd = cursor.read_u8()? ^ PACKET_INITIAL_CMD_MASK;
181184
let Ok(cmd) = cmd.try_into() else {
@@ -188,9 +191,13 @@ impl HidMessageParser {
188191
let expected_size = cursor.read_u16::<BigEndian>()?;
189192

190193
let mut payload = vec![];
191-
payload.extend(&self.packets[0][PACKET_INITIAL_HEADER_SIZE..]);
192-
for cont_packet in &self.packets[1..] {
193-
payload.extend_from_slice(&cont_packet[PACKET_CONT_HEADER_SIZE..]);
194+
if let Some(initial_payload) = initial.get(PACKET_INITIAL_HEADER_SIZE..) {
195+
payload.extend(initial_payload);
196+
}
197+
for cont_packet in continuations {
198+
if let Some(cont_payload) = cont_packet.get(PACKET_CONT_HEADER_SIZE..) {
199+
payload.extend_from_slice(cont_payload);
200+
}
194201
}
195202

196203
payload.truncate(expected_size as usize);

0 commit comments

Comments
 (0)