Vulnerability to attacks because there is no size limit

[BijanVan]

tokio/tokio-io/src/lines.rs

Line 48 in 687871d

let n = try_nb!(self.io.read_line(&mut self.line));

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.

[carllerche]

I agree, this should probably be fixed!

[BijanVan]

I think BufRead requires something like:
fn read_line(&mut self, buf: &mut String, max_length: usize) -> Result
in addition to:
fn read_line(&mut self, buf: &mut String) -> Result

https://doc.rust-lang.org/stable/std/io/trait.BufRead.html#method.read_line

[carllerche]

I would probably not use the impl in std and write our own.

[kpp]

This issue should be created in futures repo, because they copied Lines combinator: https://github.com/rust-lang-nursery/futures-rs/blob/master/futures-util/src/io/lines.rs#L48

[carllerche]

My guess is that the futures team is not interested in this change as they are focused on mirroring std and the issue exists in std as well.

/cc @cramertj @aturon

If it isn't resolved there, Tokio can provide an alternate, more robust, implementation.

[cramertj]

IMO both std and futures should probably also offer a read_line_max or similar that allows specifying a maximum line size. I think it makes sense to offer this (or similar) functionality on the tokio side for now.

[jasondavies]

It sounds like the best way forward might be to add an optional max_line_length parameter to the lines combinator in futures-util? It seems that std will not add read_line_max (from the above linked issue) so I think Read::take could be used instead.

I'm not clear on why the same lines utility combinator exists in tokio-io and futures-util -- is the former going to be dropped?

[BijanVan]

This could be a possible solution:

use bytes::BytesMut;
use futures::{Async, Poll, Stream};
use std::io::{self, BufRead};

use AsyncRead;

/// Combinator created by the top-level `lines` method which is a stream over
/// the lines of text on an I/O object.
#[derive(Debug)]
pub struct Lines<A> {
    io: A,
    line: BytesMut,
    max_length: usize,
}

/// Creates a new stream from the I/O object given representing the lines of
/// input that are found on `A`.
///
/// This method takes an asynchronous I/O object, `a`, and returns a `Stream` of
/// lines that the object contains. The returned stream will reach its end once
/// `a` reaches EOF.
pub fn lines_with_max_length<A>(a: A, max_length: usize) -> Lines<A>
where
    A: AsyncRead + BufRead,
{
    Lines {
        io: a,
        line: BytesMut::with_capacity(max_length),
        max_length: max_length,
    }
}

impl<A> Lines<A> {
    /// Returns the underlying I/O object.
    ///
    /// Note that this may lose data already read into internal buffers. It's
    /// recommended to only call this once the stream has reached its end.
    pub fn into_inner(self) -> A {
        self.io
    }
}

impl<A> Stream for Lines<A>
where
    A: AsyncRead + BufRead,
{
    type Item = String;
    type Error = io::Error;

    fn poll(&mut self) -> Poll<Option<String>, io::Error> {
        loop {
            if self.line.len() > self.max_length {
                return Err(io::Error::new(
                    io::ErrorKind::Other,
                    "Maximum length error.",
                ));
            }

            let n = try_ready!(self.io.read_buf(&mut self.line));
            if n == 0 && self.line.len() == 0 {
                return Ok(None.into());
            }

            let pos = self
                .line
                .windows(2)
                .enumerate()
                .find(|&(_, bytes)| bytes == b"\r\n")
                .map(|(i, _)| i);

            if let Some(pos) = pos {
                let mut line = self.line.split_to(pos + 2);
                line.split_off(pos);
                return Ok(Async::Ready(Some(
                    String::from_utf8_lossy(&line).to_string(),
                )));
            }
        }
    }
}

[carllerche]

This should be possible to add to LinesCodec once #360 lands.