Skip to content

Commit 0453761

Browse files
committed
Make sure that bytes passed into creating a new NinaParam* do not exceed the type's max size.
1 parent 46f0389 commit 0453761

File tree

2 files changed

+122
-29
lines changed

2 files changed

+122
-29
lines changed

esp32-wroom-rp/src/protocol.rs

Lines changed: 74 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,17 @@ pub(crate) enum NinaCommand {
4343
SendDataTcp = 0x44,
4444
}
4545

46-
pub(crate) trait NinaConcreteParam {
46+
pub(crate) trait NinaConcreteParam
47+
where
48+
Self: core::marker::Sized,
49+
{
4750
type DataBuffer;
4851
// Length of parameter in bytes
4952
type LengthAsBytes: IntoIterator<Item = u8>;
5053

5154
fn new(data: &str) -> Self;
5255

53-
fn from_bytes(bytes: &[u8]) -> Self;
56+
fn from_bytes(bytes: &[u8]) -> Result<Self, Error>;
5457

5558
fn data(&self) -> &[u8];
5659

@@ -176,14 +179,17 @@ impl NinaConcreteParam for NinaByteParam {
176179
}
177180
}
178181

179-
fn from_bytes(bytes: &[u8]) -> Self {
180-
let mut data_as_bytes: Self::DataBuffer = Vec::new();
182+
fn from_bytes(bytes: &[u8]) -> Result<Self, Error> {
183+
if bytes.len() > MAX_NINA_BYTE_PARAM_BUFFER_LENGTH {
184+
return Err(ProtocolError::TooManyParameters.into());
185+
}
181186

187+
let mut data_as_bytes: Self::DataBuffer = Vec::new();
182188
data_as_bytes.extend_from_slice(bytes).unwrap_or_default();
183-
Self {
189+
Ok(Self {
184190
length: data_as_bytes.len() as u8,
185191
data: data_as_bytes,
186-
}
192+
})
187193
}
188194

189195
fn data(&self) -> &[u8] {
@@ -199,6 +205,15 @@ impl NinaConcreteParam for NinaByteParam {
199205
}
200206
}
201207

208+
impl Default for NinaByteParam {
209+
fn default() -> Self {
210+
Self {
211+
length: 0,
212+
data: Vec::new(),
213+
}
214+
}
215+
}
216+
202217
impl NinaConcreteParam for NinaWordParam {
203218
type DataBuffer = Vec<u8, MAX_NINA_WORD_PARAM_BUFFER_LENGTH>;
204219
type LengthAsBytes = [u8; 1];
@@ -211,13 +226,17 @@ impl NinaConcreteParam for NinaWordParam {
211226
}
212227
}
213228

214-
fn from_bytes(bytes: &[u8]) -> Self {
229+
fn from_bytes(bytes: &[u8]) -> Result<Self, Error> {
230+
if bytes.len() > MAX_NINA_WORD_PARAM_BUFFER_LENGTH {
231+
return Err(ProtocolError::TooManyParameters.into());
232+
}
233+
215234
let mut data_as_bytes: Self::DataBuffer = Vec::new();
216235
data_as_bytes.extend_from_slice(bytes).unwrap_or_default();
217-
Self {
236+
Ok(Self {
218237
length: data_as_bytes.len() as u8,
219238
data: data_as_bytes,
220-
}
239+
})
221240
}
222241

223242
fn data(&self) -> &[u8] {
@@ -233,6 +252,15 @@ impl NinaConcreteParam for NinaWordParam {
233252
}
234253
}
235254

255+
impl Default for NinaWordParam {
256+
fn default() -> Self {
257+
Self {
258+
length: 0,
259+
data: Vec::new(),
260+
}
261+
}
262+
}
263+
236264
impl NinaConcreteParam for NinaSmallArrayParam {
237265
type DataBuffer = Vec<u8, MAX_NINA_SMALL_ARRAY_PARAM_BUFFER_LENGTH>;
238266
type LengthAsBytes = [u8; 1];
@@ -245,13 +273,17 @@ impl NinaConcreteParam for NinaSmallArrayParam {
245273
}
246274
}
247275

248-
fn from_bytes(bytes: &[u8]) -> Self {
276+
fn from_bytes(bytes: &[u8]) -> Result<Self, Error> {
277+
if bytes.len() > MAX_NINA_SMALL_ARRAY_PARAM_BUFFER_LENGTH {
278+
return Err(ProtocolError::TooManyParameters.into());
279+
}
280+
249281
let mut data_as_bytes: Self::DataBuffer = Vec::new();
250282
data_as_bytes.extend_from_slice(bytes).unwrap_or_default();
251-
Self {
283+
Ok(Self {
252284
length: data_as_bytes.len() as u8,
253285
data: data_as_bytes,
254-
}
286+
})
255287
}
256288

257289
fn data(&self) -> &[u8] {
@@ -267,6 +299,15 @@ impl NinaConcreteParam for NinaSmallArrayParam {
267299
}
268300
}
269301

302+
impl Default for NinaSmallArrayParam {
303+
fn default() -> Self {
304+
Self {
305+
length: 0,
306+
data: Vec::new(),
307+
}
308+
}
309+
}
310+
270311
impl NinaConcreteParam for NinaLargeArrayParam {
271312
type DataBuffer = Vec<u8, MAX_NINA_LARGE_ARRAY_PARAM_BUFFER_LENGTH>;
272313
type LengthAsBytes = [u8; 2];
@@ -279,13 +320,17 @@ impl NinaConcreteParam for NinaLargeArrayParam {
279320
}
280321
}
281322

282-
fn from_bytes(bytes: &[u8]) -> Self {
323+
fn from_bytes(bytes: &[u8]) -> Result<Self, Error> {
324+
if bytes.len() > MAX_NINA_LARGE_ARRAY_PARAM_BUFFER_LENGTH {
325+
return Err(ProtocolError::TooManyParameters.into());
326+
}
327+
283328
let mut data_as_bytes: Self::DataBuffer = Vec::new();
284329
data_as_bytes.extend_from_slice(bytes).unwrap_or_default();
285-
Self {
330+
Ok(Self {
286331
length: data_as_bytes.len() as u16,
287332
data: data_as_bytes,
288-
}
333+
})
289334
}
290335

291336
fn data(&self) -> &[u8] {
@@ -304,6 +349,15 @@ impl NinaConcreteParam for NinaLargeArrayParam {
304349
}
305350
}
306351

352+
impl Default for NinaLargeArrayParam {
353+
fn default() -> Self {
354+
Self {
355+
length: 0,
356+
data: Vec::new(),
357+
}
358+
}
359+
}
360+
307361
pub(crate) trait ProtocolInterface {
308362
fn init(&mut self);
309363
fn reset<D: DelayMs<u16>>(&mut self, delay: &mut D);
@@ -356,6 +410,9 @@ pub enum ProtocolError {
356410
InvalidNumberOfParameters,
357411
/// Too many parameters sent over the data bus.
358412
TooManyParameters,
413+
/// Payload is larger than the maximum buffer size allowed for transmission over
414+
/// the data bus.
415+
PayloadTooLarge,
359416
}
360417

361418
impl Format for ProtocolError {
@@ -365,7 +422,8 @@ impl Format for ProtocolError {
365422
ProtocolError::CommunicationTimeout => write!(fmt, "Communication with ESP32 target timed out."),
366423
ProtocolError::InvalidCommand => write!(fmt, "Encountered an invalid command while communicating with ESP32 target."),
367424
ProtocolError::InvalidNumberOfParameters => write!(fmt, "Encountered an unexpected number of parameters for a NINA command while communicating with ESP32 target."),
368-
ProtocolError::TooManyParameters => write!(fmt, "Encountered too many parameters for a NINA command while communicating with ESP32 target.")
425+
ProtocolError::TooManyParameters => write!(fmt, "Encountered too many parameters for a NINA command while communicating with ESP32 target."),
426+
ProtocolError::PayloadTooLarge => write!(fmt, "The payload is larger than the max buffer size allowed for a NINA parameter while communicating with ESP32 target."),
369427
}
370428
}
371429
}

esp32-wroom-rp/src/spi.rs

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ where
8080

8181
fn disconnect(&mut self) -> Result<(), Error> {
8282
let dummy_param = NinaByteParam::from_bytes(&[ControlByte::Dummy as u8]);
83-
let operation = Operation::new(NinaCommand::Disconnect).param(dummy_param.into());
83+
let operation =
84+
Operation::new(NinaCommand::Disconnect).param(dummy_param.unwrap_or_default().into());
8485

8586
self.execute(&operation)?;
8687

@@ -93,9 +94,17 @@ where
9394
// FIXME: refactor Operation so it can take different NinaParam types
9495
let operation = Operation::new(NinaCommand::SetDNSConfig)
9596
// FIXME: first param should be able to be a NinaByteParam:
96-
.param(NinaByteParam::from_bytes(&[1]).into())
97-
.param(NinaSmallArrayParam::from_bytes(&ip1).into())
98-
.param(NinaSmallArrayParam::from_bytes(&ip2.unwrap_or_default()).into());
97+
.param(NinaByteParam::from_bytes(&[1]).unwrap_or_default().into())
98+
.param(
99+
NinaSmallArrayParam::from_bytes(&ip1)
100+
.unwrap_or_default()
101+
.into(),
102+
)
103+
.param(
104+
NinaSmallArrayParam::from_bytes(&ip2.unwrap_or_default())
105+
.unwrap_or_default()
106+
.into(),
107+
);
99108

100109
self.execute(&operation)?;
101110

@@ -166,10 +175,26 @@ where
166175
) -> Result<(), Error> {
167176
let port_as_bytes = [((port & 0xff00) >> 8) as u8, (port & 0xff) as u8];
168177
let operation = Operation::new(NinaCommand::StartClientTcp)
169-
.param(NinaSmallArrayParam::from_bytes(&ip).into())
170-
.param(NinaWordParam::from_bytes(&port_as_bytes).into())
171-
.param(NinaByteParam::from_bytes(&[socket]).into())
172-
.param(NinaByteParam::from_bytes(&[*mode as u8]).into());
178+
.param(
179+
NinaSmallArrayParam::from_bytes(&ip)
180+
.unwrap_or_default()
181+
.into(),
182+
)
183+
.param(
184+
NinaWordParam::from_bytes(&port_as_bytes)
185+
.unwrap_or_default()
186+
.into(),
187+
)
188+
.param(
189+
NinaByteParam::from_bytes(&[socket])
190+
.unwrap_or_default()
191+
.into(),
192+
)
193+
.param(
194+
NinaByteParam::from_bytes(&[*mode as u8])
195+
.unwrap_or_default()
196+
.into(),
197+
);
173198

174199
self.execute(&operation)?;
175200

@@ -184,8 +209,11 @@ where
184209
// TODO: passing in TransportMode but not using, for now. It will become a way
185210
// of stopping the right kind of client (e.g. TCP, vs UDP)
186211
fn stop_client_tcp(&mut self, socket: Socket, _mode: &TransportMode) -> Result<(), Error> {
187-
let operation = Operation::new(NinaCommand::StopClientTcp)
188-
.param(NinaByteParam::from_bytes(&[socket]).into());
212+
let operation = Operation::new(NinaCommand::StopClientTcp).param(
213+
NinaByteParam::from_bytes(&[socket])
214+
.unwrap_or_default()
215+
.into(),
216+
);
189217

190218
self.execute(&operation)?;
191219

@@ -198,8 +226,11 @@ where
198226
}
199227

200228
fn get_client_state_tcp(&mut self, socket: Socket) -> Result<ConnectionState, Error> {
201-
let operation = Operation::new(NinaCommand::GetClientStateTcp)
202-
.param(NinaByteParam::from_bytes(&[socket]).into());
229+
let operation = Operation::new(NinaCommand::GetClientStateTcp).param(
230+
NinaByteParam::from_bytes(&[socket])
231+
.unwrap_or_default()
232+
.into(),
233+
);
203234

204235
self.execute(&operation)?;
205236

@@ -215,7 +246,11 @@ where
215246
socket: Socket,
216247
) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error> {
217248
let operation = Operation::new(NinaCommand::SendDataTcp)
218-
.param(NinaLargeArrayParam::from_bytes(&[socket]).into())
249+
.param(
250+
NinaLargeArrayParam::from_bytes(&[socket])
251+
.unwrap_or_default()
252+
.into(),
253+
)
219254
.param(NinaLargeArrayParam::new(data).into());
220255

221256
self.execute(&operation)?;

0 commit comments

Comments
 (0)