Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that in Window::new, the bytes read returned from the reader are used to set_len:
|
let mut front = vec!(0;block_size); |
|
let mut back = vec!(0;block_size); |
|
let size = try!(reader.read(front.as_mut_slice())); |
|
unsafe { |
|
front.set_len(size); |
|
} |
|
let size = try!(reader.read(back.as_mut_slice())); |
|
unsafe { |
|
back.set_len(size); |
|
} |
This means that a buggy Read implementation that returns more bytes than the buf size can cause front and back to contain initialized memory. See this example:
#![forbid(unsafe_code)]
use rdiff::BlockHashes;
use std::io::{Cursor, Read};
struct MyRead {
first: bool,
}
impl MyRead {
pub fn new() -> Self {
MyRead { first: false }
}
}
impl Read for MyRead {
fn read(&mut self, _buf: &mut [u8]) -> std::io::Result<usize> {
if !self.first {
self.first = true;
// First iteration: return more than the buffer size
Ok(256)
} else {
// Second iteration: indicate that we are done
Ok(0)
}
}
}
fn main() {
let mut hashes = BlockHashes::new(Cursor::new("Hello"), 32).unwrap();
let diff = hashes.diff_and_update(MyRead::new()).unwrap();
for insert in diff.inserts() {
println!("{:?}", insert);
}
}
This outputs:
Insert(0, '1���� =�>�U��X���������������X�q')
I think there should be an assert in Window::new to ensure that the number of bytes are <= block_size
Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that in
Window::new, the bytes read returned from thereaderare used toset_len:rdiff/src/window.rs
Lines 9 to 18 in 6680843
This means that a buggy
Readimplementation that returns more bytes than the buf size can causefrontandbackto contain initialized memory. See this example:This outputs:
I think there should be an assert in
Window::newto ensure that the number of bytes are<= block_size