Rust: Vulnerability to attacks because there is no size limit

Created on 6 Mar 2018  路  8Comments  路  Source: rust-lang/rust

https://github.com/rust-lang/rust/blob/2789b067da2ac921b86199bde21dd231ace1da39/src/libstd/io/stdio.rs#L274

read_line function seems vulnerable to attack. If an attacker sends data continually without "\r\n" the size of buffer (String) would keep growing without a limit.
tokio

C-enhancement E-medium P-medium T-doc T-libs

Most helpful comment

That should be done via Read::take, so I'm not sure that it's entirely useful. Anyway, it almost seems like this is a too specific thing to add - in most cases, I'd imagine that if you want to limit the potential line length, you'd want a different return type, among maybe other things. Since I'd imagine they'd be fairly specific to each use case it seems easier for users to implement that on their own via read or a similar call.

All 8 comments

This is a function on the standard input though? What is the threat model? That you DoS your own computer?

That is not limited to Tokio. TcpStream from standard library could be wrapped by BufReader.
Here is an example:

`use std::io::{BufRead, BufReader};
use std::net::TcpListener;

fn main() {
let listener = TcpListener::bind("localhost:8080").unwrap();

for stream in listener.incoming() {
    let mut stream = stream.unwrap();
    let mut string_buf = String::new();
    let mut reader = BufReader::new(stream);
    let line = reader.read_line(&mut string_buf);
}

}
`

This somewhat feels like something the user should be aware of. It's also not clear to me that there's much we can/should do about this -- an arbitrary limit on our side would be decidedly odd (and potentially break some use cases). Users using and of the read methods should be aware that they are both blocking and may not return (ever, potentially). This seems semi-obvious to me... but perhaps we could clarify it in documentation.

I guess adding
fn read_line(&mut self, buf: &mut String, max_length: usize) -> Result
in addition to:
fn read_line(&mut self, buf: &mut String) -> Result
would not break anything.

That should be done via Read::take, so I'm not sure that it's entirely useful. Anyway, it almost seems like this is a too specific thing to add - in most cases, I'd imagine that if you want to limit the potential line length, you'd want a different return type, among maybe other things. Since I'd imagine they'd be fairly specific to each use case it seems easier for users to implement that on their own via read or a similar call.

We discussed this during libs triage today and the conclusion was that we didn't consider this a bug to fix in libstd but would of course be more than willing to update the documentation. As a result I'm reclassifying as a documentation issue.

Note to future selves: if we put a note about this on BufRead::read_line, we should consider adding a note on BufRead::read_until too since it's susceptible to the same issue

If the added doc is deemed sufficient, this should be closed, though I do not know who to ping for that.

Was this page helpful?
0 / 5 - 0 ratings