Skip to content

Commit 15381a2

Browse files
committed
Fix panics found due to fuzzing
1 parent 33914ac commit 15381a2

9 files changed

+94
-36
lines changed

tunshell-client/src/shell/proto.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ impl Message for ShellClientMessage {
5555
Self::Error(payload) => payload.as_bytes().to_vec(),
5656
};
5757

58-
Ok(RawMessage::new(self.type_id(), buff))
58+
RawMessage::new(self.type_id(), buff)
5959
}
6060

6161
fn deserialise(raw_message: &RawMessage) -> Result<Self> {
@@ -97,7 +97,7 @@ impl Message for ShellServerMessage {
9797
Self::Error(payload) => payload.as_bytes().to_vec(),
9898
};
9999

100-
Ok(RawMessage::new(self.type_id(), buff))
100+
RawMessage::new(self.type_id(), buff)
101101
}
102102

103103
fn deserialise(raw_message: &RawMessage) -> Result<Self> {
@@ -137,7 +137,7 @@ mod tests {
137137
let message = ShellClientMessage::Key("key".to_owned());
138138
let serialised = message.serialise().unwrap();
139139

140-
assert_eq!(serialised, RawMessage::new(1, "key".as_bytes().to_vec()));
140+
assert_eq!(serialised, RawMessage::new(1, "key".as_bytes().to_vec()).unwrap());
141141

142142
let deserialised = ShellClientMessage::deserialise(&serialised).unwrap();
143143

@@ -170,7 +170,7 @@ mod tests {
170170
let message = ShellClientMessage::Stdin(vec![1, 2, 3, 4, 5]);
171171
let serialised = message.serialise().unwrap();
172172

173-
assert_eq!(serialised, RawMessage::new(3, vec![1, 2, 3, 4, 5]));
173+
assert_eq!(serialised, RawMessage::new(3, vec![1, 2, 3, 4, 5]).unwrap());
174174

175175
let deserialised = ShellClientMessage::deserialise(&serialised).unwrap();
176176

@@ -197,7 +197,7 @@ mod tests {
197197
let message = ShellServerMessage::KeyAccepted;
198198
let serialised = message.serialise().unwrap();
199199

200-
assert_eq!(serialised, RawMessage::new(1, vec![]));
200+
assert_eq!(serialised, RawMessage::new(1, vec![]).unwrap());
201201

202202
let deserialised = ShellServerMessage::deserialise(&serialised).unwrap();
203203

@@ -209,7 +209,7 @@ mod tests {
209209
let message = ShellServerMessage::KeyRejected;
210210
let serialised = message.serialise().unwrap();
211211

212-
assert_eq!(serialised, RawMessage::new(2, vec![]));
212+
assert_eq!(serialised, RawMessage::new(2, vec![]).unwrap());
213213

214214
let deserialised = ShellServerMessage::deserialise(&serialised).unwrap();
215215

@@ -221,7 +221,7 @@ mod tests {
221221
let message = ShellServerMessage::Stdout(vec![1, 2, 3, 4, 5]);
222222
let serialised = message.serialise().unwrap();
223223

224-
assert_eq!(serialised, RawMessage::new(3, vec![1, 2, 3, 4, 5]));
224+
assert_eq!(serialised, RawMessage::new(3, vec![1, 2, 3, 4, 5]).unwrap());
225225

226226
let deserialised = ShellServerMessage::deserialise(&serialised).unwrap();
227227

@@ -233,7 +233,7 @@ mod tests {
233233
let message = ShellServerMessage::Exited(5);
234234
let serialised = message.serialise().unwrap();
235235

236-
assert_eq!(serialised, RawMessage::new(4, vec![5]));
236+
assert_eq!(serialised, RawMessage::new(4, vec![5]).unwrap());
237237

238238
let deserialised = ShellServerMessage::deserialise(&serialised).unwrap();
239239

@@ -245,7 +245,7 @@ mod tests {
245245
let message = ShellServerMessage::Error("test".to_owned());
246246
let serialised = message.serialise().unwrap();
247247

248-
assert_eq!(serialised, RawMessage::new(255, "test".as_bytes().to_vec()));
248+
assert_eq!(serialised, RawMessage::new(255, "test".as_bytes().to_vec()).unwrap());
249249

250250
let deserialised = ShellServerMessage::deserialise(&serialised).unwrap();
251251

tunshell-client/src/stream/aes_stream.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl Message for EncryptedMessage {
3636
.unwrap();
3737
cursor.write(self.ciphertext.as_slice()).unwrap();
3838

39-
Ok(RawMessage::new(self.type_id(), cursor.into_inner()))
39+
RawMessage::new(self.type_id(), cursor.into_inner())
4040
}
4141

4242
fn deserialise(raw_message: &RawMessage) -> Result<Self> {

tunshell-shared/fuzz.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@ set -e
55
export CARGO_TARGET_DIR="$PWD/fuzz/target"
66

77
cargo afl build
8-
cargo afl fuzz -i fuzz/corpus -o fuzz/out $CARGO_TARGET_DIR/debug/fuzz_client
8+
cargo afl fuzz -i fuzz/corpus -o fuzz/out $CARGO_TARGET_DIR/debug/fuzzing

tunshell-shared/fuzz/corpus/id:000000,sig:06,src:001034,time:624166,op:havoc,rep:128

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.
Binary file not shown.

tunshell-shared/fuzz/corpus/id:000002,sig:06,src:001054,time:645037,op:havoc,rep:128

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.
Binary file not shown.

tunshell-shared/src/message.rs

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use anyhow::{anyhow, Result};
1+
use anyhow::{anyhow, Error, Result};
22
use serde::{Deserialize, Serialize};
33

44
#[derive(Debug, PartialEq, Clone)]
@@ -65,14 +65,21 @@ pub struct RelayPayload {
6565
}
6666

6767
impl RawMessage {
68-
pub fn new(type_id: u8, data: Vec<u8>) -> Self {
68+
pub fn new(type_id: u8, data: Vec<u8>) -> Result<Self> {
69+
if data.len() > i16::MAX as usize {
70+
return Err(Error::msg(format!(
71+
"message length ({}) cannot be greater than {}",
72+
data.len(),
73+
i16::MAX
74+
)));
75+
}
6976
assert!(data.len() <= i16::MAX as usize);
7077

71-
Self {
78+
Ok(Self {
7279
type_id,
7380
length: data.len() as u16,
7481
data,
75-
}
82+
})
7683
}
7784

7885
pub fn type_id(&self) -> u8 {
@@ -125,8 +132,8 @@ impl Message for ServerMessage {
125132
Self::StartRelayMode => vec![],
126133
Self::Relay(payload) => payload.data.clone(),
127134
};
128-
129-
Ok(RawMessage::new(type_id, data))
135+
136+
RawMessage::new(type_id, data)
130137
}
131138

132139
fn deserialise(raw_message: &RawMessage) -> Result<Self> {
@@ -210,7 +217,7 @@ impl Message for ClientMessage {
210217
Self::Relay(payload) => payload.data.clone(),
211218
};
212219

213-
Ok(RawMessage::new(type_id, data))
220+
RawMessage::new(type_id, data)
214221
}
215222

216223
fn deserialise(raw_message: &RawMessage) -> Result<Self> {
@@ -256,7 +263,7 @@ mod tests {
256263

257264
#[test]
258265
fn test_raw_message_to_vec() {
259-
let raw_message = RawMessage::new(0, vec![1, 2, 3]);
266+
let raw_message = RawMessage::new(0, vec![1, 2, 3]).unwrap();
260267

261268
let vec = raw_message.to_vec();
262269

@@ -407,7 +414,7 @@ mod tests {
407414

408415
#[test]
409416
fn test_server_deserialise_close() {
410-
let raw_message = RawMessage::new(0, vec![]);
417+
let raw_message = RawMessage::new(0, vec![]).unwrap();
411418

412419
let message = ServerMessage::deserialise(&raw_message).unwrap();
413420

@@ -416,7 +423,7 @@ mod tests {
416423

417424
#[test]
418425
fn test_server_deserialise_key_accepted() {
419-
let raw_message = RawMessage::new(1, vec![]);
426+
let raw_message = RawMessage::new(1, vec![]).unwrap();
420427

421428
let message = ServerMessage::deserialise(&raw_message).unwrap();
422429

@@ -425,7 +432,7 @@ mod tests {
425432

426433
#[test]
427434
fn test_server_deserialise_key_rejected() {
428-
let raw_message = RawMessage::new(2, vec![]);
435+
let raw_message = RawMessage::new(2, vec![]).unwrap();
429436

430437
let message = ServerMessage::deserialise(&raw_message).unwrap();
431438

@@ -434,7 +441,7 @@ mod tests {
434441

435442
#[test]
436443
fn test_server_deserialise_already_joined() {
437-
let raw_message = RawMessage::new(3, vec![]);
444+
let raw_message = RawMessage::new(3, vec![]).unwrap();
438445

439446
let message = ServerMessage::deserialise(&raw_message).unwrap();
440447

@@ -446,7 +453,7 @@ mod tests {
446453
let raw_message = RawMessage::new(
447454
4,
448455
Vec::from(r#"{"peer_key": "key", "peer_ip_address": "123.123.123.123", "session_nonce": "nonce"}"#.as_bytes()),
449-
);
456+
).unwrap();
450457

451458
let message = ServerMessage::deserialise(&raw_message).unwrap();
452459

@@ -462,7 +469,7 @@ mod tests {
462469

463470
#[test]
464471
fn test_server_deserialise_peer_left() {
465-
let raw_message = RawMessage::new(5, vec![]);
472+
let raw_message = RawMessage::new(5, vec![]).unwrap();
466473

467474
let message = ServerMessage::deserialise(&raw_message).unwrap();
468475

@@ -471,7 +478,7 @@ mod tests {
471478

472479
#[test]
473480
fn test_server_deserialise_bind_for_direct_connect() {
474-
let raw_message = RawMessage::new(6, vec![]);
481+
let raw_message = RawMessage::new(6, vec![]).unwrap();
475482

476483
let message = ServerMessage::deserialise(&raw_message).unwrap();
477484

@@ -483,7 +490,8 @@ mod tests {
483490
let raw_message = RawMessage::new(
484491
7,
485492
Vec::from(r#"{"tcp_port":12345,"udp_port":123}"#.as_bytes()),
486-
);
493+
)
494+
.unwrap();
487495

488496
let message = ServerMessage::deserialise(&raw_message).unwrap();
489497

@@ -498,7 +506,7 @@ mod tests {
498506

499507
#[test]
500508
fn test_server_deserialise_start_relay_mode() {
501-
let raw_message = RawMessage::new(8, vec![]);
509+
let raw_message = RawMessage::new(8, vec![]).unwrap();
502510

503511
let message = ServerMessage::deserialise(&raw_message).unwrap();
504512

@@ -507,7 +515,7 @@ mod tests {
507515

508516
#[test]
509517
fn test_server_deserialise_relay() {
510-
let raw_message = RawMessage::new(9, vec![1, 2, 3, 4, 5, 6, 7, 8, 9]);
518+
let raw_message = RawMessage::new(9, vec![1, 2, 3, 4, 5, 6, 7, 8, 9]).unwrap();
511519

512520
let message = ServerMessage::deserialise(&raw_message).unwrap();
513521

@@ -597,7 +605,7 @@ mod tests {
597605

598606
#[test]
599607
fn test_client_deserialise_close() {
600-
let raw_message = RawMessage::new(0, vec![]);
608+
let raw_message = RawMessage::new(0, vec![]).unwrap();
601609

602610
let message = ClientMessage::deserialise(&raw_message).unwrap();
603611

@@ -606,7 +614,7 @@ mod tests {
606614

607615
#[test]
608616
fn test_client_deserialise_key() {
609-
let raw_message = RawMessage::new(1, Vec::from(r#"{"key":"key"}"#.as_bytes()));
617+
let raw_message = RawMessage::new(1, Vec::from(r#"{"key":"key"}"#.as_bytes())).unwrap();
610618

611619
let message = ClientMessage::deserialise(&raw_message).unwrap();
612620

@@ -623,7 +631,8 @@ mod tests {
623631
let raw_message = RawMessage::new(
624632
2,
625633
Vec::from(r#"{"tcp_port":null,"udp_port":2222}"#.as_bytes()),
626-
);
634+
)
635+
.unwrap();
627636

628637
let message = ClientMessage::deserialise(&raw_message).unwrap();
629638

@@ -638,7 +647,7 @@ mod tests {
638647

639648
#[test]
640649
fn test_client_deserialise_direct_contact_succeeded() {
641-
let raw_message = RawMessage::new(3, vec![]);
650+
let raw_message = RawMessage::new(3, vec![]).unwrap();
642651

643652
let message = ClientMessage::deserialise(&raw_message).unwrap();
644653

@@ -647,7 +656,7 @@ mod tests {
647656

648657
#[test]
649658
fn test_client_deserialise_direct_contact_failed() {
650-
let raw_message = RawMessage::new(4, vec![]);
659+
let raw_message = RawMessage::new(4, vec![]).unwrap();
651660

652661
let message = ClientMessage::deserialise(&raw_message).unwrap();
653662

@@ -656,7 +665,7 @@ mod tests {
656665

657666
#[test]
658667
fn test_client_deserialise_relay() {
659-
let raw_message = RawMessage::new(5, vec![1, 2, 3, 4, 5]);
668+
let raw_message = RawMessage::new(5, vec![1, 2, 3, 4, 5]).unwrap();
660669

661670
let message = ClientMessage::deserialise(&raw_message).unwrap();
662671

tunshell-shared/src/message_stream.rs

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,23 @@ impl<I: Message, O: Message, S: AsyncRead + AsyncWrite + Unpin> Stream for Messa
138138
.collect(),
139139
);
140140

141+
if let Err(err) = raw_message {
142+
debug!("Could not parse message {:?}", err);
143+
self.closed = true;
144+
145+
return Poll::Ready(Some(Err(err)));
146+
}
147+
141148
self.read_buff.drain(..3 + message_length);
142149

143-
let result = match O::deserialise(&raw_message) {
150+
let result = match O::deserialise(&raw_message.unwrap()) {
144151
Ok(message) => {
145152
debug!("Received message {:?}", message);
146153
Ok(message)
147154
}
148155
Err(err) => {
149156
debug!("Error while deserialised received message {:?}", err);
157+
self.closed = true;
150158
Err(err)
151159
}
152160
};
@@ -230,6 +238,7 @@ mod tests {
230238
use super::*;
231239
use futures::executor;
232240
use futures::io::Cursor;
241+
use std::io::Read;
233242

234243
#[test]
235244
fn test_read_empty_stream() {
@@ -409,4 +418,42 @@ mod tests {
409418
assert_eq!(result.is_none(), true);
410419
assert_eq!(stream.closed, true);
411420
}
421+
422+
#[test]
423+
fn test_fuzz_corpus() {
424+
let corpus_dir = std::path::Path::new(&std::env::current_exe().unwrap())
425+
.parent()
426+
.unwrap()
427+
.parent()
428+
.unwrap()
429+
.parent()
430+
.unwrap()
431+
.parent()
432+
.unwrap()
433+
.join("tunshell-shared/fuzz/corpus");
434+
435+
let files = std::fs::read_dir(corpus_dir)
436+
.unwrap()
437+
.map(|i| i.unwrap().path())
438+
.filter(|i| i.is_file());
439+
440+
for path in files {
441+
let mut file = std::fs::File::open(path).unwrap();
442+
let mut input = Vec::<u8>::new();
443+
file.read_to_end(&mut input).unwrap();
444+
445+
// Ensure no crashes while parsing
446+
let input = Cursor::new(input);
447+
448+
let stream_server =
449+
MessageStream::<ClientMessage, ServerMessage, Cursor<Vec<u8>>>::new(input.clone());
450+
let stream_client =
451+
MessageStream::<ServerMessage, ClientMessage, Cursor<Vec<u8>>>::new(input);
452+
453+
let _ =
454+
executor::block_on_stream(stream_server).collect::<Vec<Result<ServerMessage>>>();
455+
let _ =
456+
executor::block_on_stream(stream_client).collect::<Vec<Result<ClientMessage>>>();
457+
}
458+
}
412459
}

0 commit comments

Comments
 (0)