Skip to content

Commit 423844d

Browse files
committed
Correct EOF handling in stream read in ChaChaDualPolyReadAdapter
When `ChaChaDualPolyReadAdapter` encounters an EOF (`Read::read` returns `Ok(0)`) while trying to drain the stream (even though the `FixedLengthReader` thinks it has available space) we'll end up infinite-looping trying to drain the stream looking for `Read::read` to return an `Err` (which it won't). The fix is, of course, simple, to detect the EOF signal. Found by the `onion_message_target` fuzzer which @dergoegge ran. Thanks to @morehouse for digging deeper on the specific fuzz test case and thoroughly reporting the underlying causes. Fixes #4139.
1 parent 7439528 commit 423844d

File tree

1 file changed

+20
-1
lines changed

1 file changed

+20
-1
lines changed

lightning/src/crypto/streams.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,10 @@ impl<T: Readable> LengthReadableArgs<([u8; 32], [u8; 32])> for ChaChaDualPolyRea
130130
let readable: T = Readable::read(&mut chacha_stream)?;
131131
while chacha_stream.read.bytes_remain() {
132132
let mut buf = [0; 256];
133-
chacha_stream.read(&mut buf)?;
133+
if chacha_stream.read(&mut buf)? == 0 {
134+
// Reached EOF
135+
return Err(DecodeError::ShortRead);
136+
}
134137
}
135138

136139
let read_len = chacha_stream.read_len;
@@ -344,4 +347,20 @@ mod tests {
344347
// This also serves to test the `option: $trait` variant of the `_decode_tlv` ser macro.
345348
do_chacha_stream_adapters_ser_macros().unwrap()
346349
}
350+
351+
#[test]
352+
fn short_read_chacha_dual_read_adapter() {
353+
// Previously, if we attempted to read from a ChaChaDualPolyReadAdapter but the object
354+
// being read is shorter than the available buffer while the buffer passed to
355+
// ChaChaDualPolyReadAdapter itself always thinks it has room, we'd end up
356+
// infinite-looping as we didn't handle `Read::read`'s 0 return values at EOF.
357+
let mut stream = &[0; 1024][..];
358+
let mut too_long_stream = FixedLengthReader::new(&mut stream, 2048);
359+
let keys = ([42; 32], [99; 32]);
360+
let res = super::ChaChaDualPolyReadAdapter::<u8>::read(&mut too_long_stream, keys);
361+
match res {
362+
Ok(_) => panic!(),
363+
Err(e) => assert_eq!(e, DecodeError::ShortRead),
364+
}
365+
}
347366
}

0 commit comments

Comments
 (0)