Skip to content

Commit ce48709

Browse files
authored
Rollup merge of #80895 - sfackler:read-to-end-ub, r=m-ou-se
Fix handling of malicious Readers in read_to_end A malicious `Read` impl could return overly large values from `read`, which would result in the guard's drop impl setting the buffer's length to greater than its capacity! ~~To fix this, the drop impl now uses the safe `truncate` function instead of `set_len` which ensures that this will not happen. The result of calling the function will be nonsensical, but that's fine given the contract violation of the `Read` impl.~~ ~~The `Guard` type is also used by `append_to_string` which does not pass untrusted values into the length field, so I've copied the guard type into each function and only modified the one used by `read_to_end`. We could just keep a single one and modify it, but it seems a bit cleaner to keep the guard code close to the functions and related specifically to them.~~ To fix this, we now assert that the returned length is not larger than the buffer passed to the method. For reference, this bug has been present for ~2.5 years since 1.20: ecbb896. Closes #80894.
2 parents 9e9aba8 + e6c07b0 commit ce48709

File tree

1 file changed

+10
-12
lines changed

1 file changed

+10
-12
lines changed

library/std/src/io/mod.rs

+10-12
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,6 @@ where
366366
{
367367
let start_len = buf.len();
368368
let mut g = Guard { len: buf.len(), buf };
369-
let ret;
370369
loop {
371370
if g.len == g.buf.len() {
372371
unsafe {
@@ -385,21 +384,20 @@ where
385384
}
386385
}
387386

388-
match r.read(&mut g.buf[g.len..]) {
389-
Ok(0) => {
390-
ret = Ok(g.len - start_len);
391-
break;
387+
let buf = &mut g.buf[g.len..];
388+
match r.read(buf) {
389+
Ok(0) => return Ok(g.len - start_len),
390+
Ok(n) => {
391+
// We can't allow bogus values from read. If it is too large, the returned vec could have its length
392+
// set past its capacity, or if it overflows the vec could be shortened which could create an invalid
393+
// string if this is called via read_to_string.
394+
assert!(n <= buf.len());
395+
g.len += n;
392396
}
393-
Ok(n) => g.len += n,
394397
Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
395-
Err(e) => {
396-
ret = Err(e);
397-
break;
398-
}
398+
Err(e) => return Err(e),
399399
}
400400
}
401-
402-
ret
403401
}
404402

405403
pub(crate) fn default_read_vectored<F>(read: F, bufs: &mut [IoSliceMut<'_>]) -> Result<usize>

0 commit comments

Comments
 (0)