reject CR and LF in basic_fields field name and value

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<Allocator>::
 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<Allocator>::
 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<Allocator>::
 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<off_t>(sname.size() + 2);

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"));

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<true> 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<false> 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();
     }
 };