diff --git a/include/boost/beast/http/impl/fields.hpp b/include/boost/beast/http/impl/fields.hpp index b42a3ccdd0..f0e86d52a0 100644 --- a/include/boost/beast/http/impl/fields.hpp +++ b/include/boost/beast/http/impl/fields.hpp @@ -775,6 +775,17 @@ keep_alive_impl( beast::detail::temporary_buffer& s, string_view value, unsigned version, bool keep_alive); +// Two single-octet finds lower to memchr, which is +// considerably faster than find_first_of("\r\n"). +inline +bool +contains_crlf(string_view s) +{ + return + s.find('\r') != string_view::npos || + s.find('\n') != string_view::npos; +} + } // detail //------------------------------------------------------------------------------ @@ -862,6 +873,11 @@ void basic_fields:: set_method_impl(string_view s) { + // A CR or LF here splits the serialized start + // line, just as it does in a field (rfc7230 3.1.1). + if(detail::contains_crlf(s)) + BOOST_THROW_EXCEPTION( + system_error{error::bad_method}); realloc_string(method_, s); } @@ -871,6 +887,9 @@ void basic_fields:: set_target_impl(string_view s) { + if(detail::contains_crlf(s)) + BOOST_THROW_EXCEPTION( + system_error{error::bad_target}); realloc_target( target_or_reason_, s); } @@ -881,6 +900,9 @@ void basic_fields:: set_reason_impl(string_view s) { + if(detail::contains_crlf(s)) + BOOST_THROW_EXCEPTION( + system_error{error::bad_reason}); realloc_string( target_or_reason_, s); } @@ -981,6 +1003,20 @@ try_create_new_element( BOOST_BEAST_ASSIGN_EC(ec, error::header_field_value_too_large); return nullptr; } + // A CR or LF in the name or value is written verbatim into the + // serialized header block, which is terminated by CRLF; either one + // splits the field and injects additional header lines (rfc7230 + // 3.2 forbids them in field-name and field-value). + if(detail::contains_crlf(sname)) + { + BOOST_BEAST_ASSIGN_EC(ec, error::bad_field); + return nullptr; + } + if(detail::contains_crlf(value)) + { + BOOST_BEAST_ASSIGN_EC(ec, error::bad_value); + return nullptr; + } value = detail::trim(value); std::uint16_t const off = static_cast(sname.size() + 2); diff --git a/test/beast/http/fields.cpp b/test/beast/http/fields.cpp index c1aa3bbe7c..8c4df3b70c 100644 --- a/test/beast/http/fields.cpp +++ b/test/beast/http/fields.cpp @@ -477,6 +477,26 @@ class fields_test : public beast::unit_test::suite BEAST_THROWS(f.set("", big_value), boost::system::system_error); } + // reject CR or LF in a field name or value (rfc7230 3.2) + { + fields f; + error_code ec; + + f.insert(field::age, "a\r\nb", "1", ec); + BEAST_EXPECT(ec == error::bad_field); + f.insert(field::age, "age", "1\r\nInjected: x", ec); + BEAST_EXPECT(ec == error::bad_value); + f.insert(field::age, "age", "1\ntwo", ec); + BEAST_EXPECT(ec == error::bad_value); + f.insert(field::age, "age", "1\rtwo", ec); + BEAST_EXPECT(ec == error::bad_value); + + BEAST_THROWS(f.set(field::location, "/\r\nSet-Cookie: x"), + boost::system::system_error); + BEAST_THROWS(f.set("X\r\nY", "1"), boost::system::system_error); + BEAST_THROWS(f.insert("X", "1\r\n2"), boost::system::system_error); + } + { fields f; BEAST_EXPECT(! f.contains("Content-Type")); diff --git a/test/beast/http/message.cpp b/test/beast/http/message.cpp index d24e130629..102849eb08 100644 --- a/test/beast/http/message.cpp +++ b/test/beast/http/message.cpp @@ -394,6 +394,27 @@ class message_test : public beast::unit_test::suite BEAST_EXPECT(h.reason() == "Not Found"); } + void + testStartLineInjection() + { + // a CR or LF in the method, target, or reason splits + // the serialized start line, the same way it does in + // a field name or value (rfc7230 3.1) + header req; + req.method_string("XYZ"); + req.target("/"); + BEAST_THROWS(req.method_string("GE\rT"), system_error); + BEAST_THROWS(req.target("/\r\nHost: evil"), system_error); + BEAST_THROWS(req.target("/\n"), system_error); + BEAST_EXPECT(req.method_string() == "XYZ"); + BEAST_EXPECT(req.target() == "/"); + + header res; + BEAST_THROWS(res.reason("OK\r\nSet-Cookie: sid=x"), + system_error); + BEAST_THROWS(res.reason("OK\r"), system_error); + } + void testNeedEof() { @@ -510,6 +531,7 @@ class message_test : public beast::unit_test::suite testMethod(); testStatus(); testReason(); + testStartLineInjection(); testNeedEof(); } };