reject CR and LF in basic_fields field name and value

[sahvx655-wq]

basic_fields::try_create_new_element assembles the on-wire field as name, ": ", value, then a terminating CRLF, but it only checks the name and value lengths, never their contents. A CR or LF sitting inside a value is copied straight into that buffer, so a header built from attacker-influenced data such as a redirect target or a reflected parameter splits the field and starts new header lines. Setting Location to "/next\r\nSet-Cookie: sid=attacker" and serialising the response puts Set-Cookie on its own line, which is classic response splitting.

The fix rejects a name or value containing CR or LF in try_create_new_element, alongside the existing size checks, reusing the bad_field and bad_value codes the parser already returns for those octets when reading. All of the set and insert overloads, including the error_code variants, route through this one function, so the check covers them together and stops the malformed field at the point it is created instead of relying on every caller to pre-scan its input.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.20%. Comparing base (56a7c3a) to head (1640608).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3105      +/-   ##
===========================================
- Coverage    93.30%   93.20%   -0.11%     
===========================================
  Files          177      177              
  Lines        13764    13739      -25     
===========================================
- Hits         12843    12805      -38     
- Misses         921      934      +13     

Files with missing lines	Coverage Δ
include/boost/beast/http/impl/fields.hpp	97.45% <100.00%> (-0.13%)	⬇️

... and 31 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56a7c3a...1640608. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ashtum]

Hi, thanks for the PR. A few caveats worth flagging:

• Given the layer Boost.Beast's serializer operates at, checks like this are usually left to a higher layer, unless doing them here proves cheaper than delegating.
• The request method, request target, and response reason-phrase aren't covered by this check and likely need the same sanitization.
• Checking for CR/LF fixes the vulnerability, but we could also validate names and values against their allowed character sets, which would cover this case as well, if it proves to be worthwhile.

Separately, core::string_view::find_first_of is oddly slow for this task. Even a naive form like:

    if( sname.find('\r') != string_view::npos ||
        sname.find('\n') != string_view::npos)
    {
        BOOST_BEAST_ASSIGN_EC(ec, error::bad_field);
        return nullptr;
    }
    if( value.find('\r') != string_view::npos ||
        value.find('\n') != string_view::npos)
    {
        BOOST_BEAST_ASSIGN_EC(ec, error::bad_value);
        return nullptr;
    }

is ~3x faster for ~30 char inputs and ~10x faster around ~300 chars. Note: I'm not suggesting this exact version, just that core::string_view::find_first_of looks like a poor fit here and warrants a closer look.

[sahvx655-wq]

Good points, took a proper look at each. On the scan: I benchmarked before touching anything, and on Apple clang with core::string_view two single-octet find() calls measure about 1.4x faster than find_first_of("\r\n") at 30 chars and roughly 5x at 300, with find_first_of only winning marginally below ~10 chars. The root cause is that find_first_of walks the input octet by octet testing membership in the needle set, while find(char) forwards to memchr and gets vectorised. I've switched to the two-find form through a small detail::contains_crlf helper and extended the same rejection to set_method_impl, set_target_impl and set_reason_impl, reusing the parser's bad_method, bad_target and bad_reason codes, with tests in message.cpp. The read path is unaffected since basic_parser never passes those strings through with a bare CR or LF; the only new cost there is one memchr pass per start line. One quirk I noticed while testing: header::method_string assigns its cached verb before calling set_method_impl, so a rejected method leaves the cached verb as unknown while the stored bytes stay intact. I left that ordering alone to keep the diff contained.

On layering, the case for doing it here is that try_create_new_element is the one choke point every set and insert overload funnels through, so the check is written once and costs two memchr passes over bytes the function is about to copy anyway. Delegating means every call site that touches request-derived data has to remember to pre-scan, and a single miss puts a split header on the wire, which is the failure mode that worries me. Full charset validation would cover this too, but it needs a per-octet table lookup rather than memchr, and its main additional effect beyond CR/LF is rejecting stray C0 controls, which are ugly but don't alter framing on a correct peer; CR and LF are the two octets that actually change message structure. If you think the table-driven version is worth that cost I'm fine doing it instead.