Skip to content

Commit e997866

Browse files
etrclaude
andcommitted
TASK-010: review-pass fixes (security: WWW-Authenticate header injection)
http_response::unauthorized() now rejects scheme/realm values containing CR, LF, or NUL with std::invalid_argument (CWE-113), and escapes embedded double-quote characters in realm per RFC 7235 §2.1 quoted-string rules (CWE-116). Without these guards a caller passing attacker-influenced input through scheme or realm could splice additional headers into the response or produce a malformed WWW-Authenticate value that downstream parsers misinterpret. Tests: 8 new LT_BEGIN_AUTO_TESTs covering CR / LF / CRLF / NUL in both scheme and realm, plus a quote-escape test asserting the backslash-escaped form `Basic realm="foo\"bar"`. All factories suite tests pass locally; cpplint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8fa2c6b commit e997866

2 files changed

Lines changed: 152 additions & 4 deletions

File tree

src/http_response.cpp

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include <map>
3232
#include <new>
3333
#include <span>
34+
#include <stdexcept>
3435
#include <string>
3536
#include <string_view>
3637
#include <type_traits>
@@ -315,15 +316,46 @@ http_response http_response::deferred(
315316
http_response http_response::unauthorized(std::string_view scheme,
316317
std::string_view realm,
317318
std::string body) {
319+
// Security: reject scheme or realm values containing CR, LF, or NUL.
320+
// Any of these characters can be used to inject additional HTTP headers
321+
// into the WWW-Authenticate response header (CWE-113). This is always a
322+
// caller error — callers must never pass untrusted user input as scheme
323+
// or realm without first validating it. Throw std::invalid_argument so
324+
// the error is visible and cannot be silently swallowed.
325+
static constexpr std::string_view kForbidden("\r\n\0", 3);
326+
if (scheme.find_first_of(kForbidden) != std::string_view::npos) {
327+
throw std::invalid_argument(
328+
"http_response::unauthorized: scheme contains forbidden control "
329+
"character (CR, LF, or NUL)");
330+
}
331+
if (realm.find_first_of(kForbidden) != std::string_view::npos) {
332+
throw std::invalid_argument(
333+
"http_response::unauthorized: realm contains forbidden control "
334+
"character (CR, LF, or NUL)");
335+
}
336+
337+
// Security: escape double-quote characters inside realm per RFC 7235
338+
// §2.1 quoted-string rules. An unescaped " terminates the quoted-string
339+
// early, producing syntactically invalid header values that some parsers
340+
// misinterpret (CWE-116).
341+
std::string escaped_realm;
342+
escaped_realm.reserve(realm.size());
343+
for (char c : realm) {
344+
if (c == '"') {
345+
escaped_realm.push_back('\\');
346+
}
347+
escaped_realm.push_back(c);
348+
}
349+
318350
http_response r;
319351
r.status_code_ = http::http_utils::http_unauthorized; // 401
320-
// Build `<scheme> realm="<realm>"`. AC #3 requires byte-for-byte
321-
// `Basic realm="myrealm"` for the canonical case.
352+
// Build `<scheme> realm="<escaped_realm>"`. AC #3 requires byte-for-byte
353+
// `Basic realm="myrealm"` for the canonical case (which has no quotes).
322354
std::string challenge;
323-
challenge.reserve(scheme.size() + realm.size() + 10);
355+
challenge.reserve(scheme.size() + escaped_realm.size() + 10);
324356
challenge.append(scheme.data(), scheme.size());
325357
challenge.append(" realm=\"", 8);
326-
challenge.append(realm.data(), realm.size());
358+
challenge.append(escaped_realm);
327359
challenge.push_back('"');
328360
r.with_header(http::http_utils::http_header_www_authenticate,
329361
challenge);

test/unit/http_response_factories_test.cpp

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
#include <cstdint>
5252
#include <functional>
5353
#include <memory>
54+
#include <stdexcept>
5455
#include <string>
5556
#include <type_traits>
5657
#include <utility>
@@ -301,6 +302,121 @@ LT_BEGIN_AUTO_TEST(http_response_factories_suite,
301302
LT_CHECK_EQ(r.get_response_code(), 401);
302303
LT_END_AUTO_TEST(unauthorized_with_explicit_body)
303304

305+
// -----------------------------------------------------------------------
306+
// unauthorized() — header injection validation (security-reviewer-iter1-1,
307+
// security-reviewer-iter1-2). CRLF sequences in scheme or realm must be
308+
// rejected (std::invalid_argument) to prevent header injection (CWE-113).
309+
// Double-quotes embedded in realm must be escaped per RFC 7235 §2.1
310+
// (backslash-escape) so the quoted-string is syntactically valid.
311+
// -----------------------------------------------------------------------
312+
LT_BEGIN_AUTO_TEST(http_response_factories_suite,
313+
unauthorized_crlf_in_scheme_throws)
314+
// CRLF in scheme must throw — caller error, not a runtime failure.
315+
bool caught = false;
316+
try {
317+
auto r = http_response::unauthorized("Basic\r\nX-Injected: hdr",
318+
"myrealm");
319+
(void)r;
320+
} catch (const std::invalid_argument&) {
321+
caught = true;
322+
}
323+
LT_CHECK_EQ(caught, true);
324+
LT_END_AUTO_TEST(unauthorized_crlf_in_scheme_throws)
325+
326+
LT_BEGIN_AUTO_TEST(http_response_factories_suite,
327+
unauthorized_lf_in_scheme_throws)
328+
bool caught = false;
329+
try {
330+
auto r = http_response::unauthorized("Basic\nEvil: hdr", "myrealm");
331+
(void)r;
332+
} catch (const std::invalid_argument&) {
333+
caught = true;
334+
}
335+
LT_CHECK_EQ(caught, true);
336+
LT_END_AUTO_TEST(unauthorized_lf_in_scheme_throws)
337+
338+
LT_BEGIN_AUTO_TEST(http_response_factories_suite,
339+
unauthorized_cr_in_scheme_throws)
340+
bool caught = false;
341+
try {
342+
auto r = http_response::unauthorized("Basic\r", "myrealm");
343+
(void)r;
344+
} catch (const std::invalid_argument&) {
345+
caught = true;
346+
}
347+
LT_CHECK_EQ(caught, true);
348+
LT_END_AUTO_TEST(unauthorized_cr_in_scheme_throws)
349+
350+
LT_BEGIN_AUTO_TEST(http_response_factories_suite,
351+
unauthorized_nul_in_scheme_throws)
352+
// NUL in scheme is equally dangerous — reject it.
353+
bool caught = false;
354+
try {
355+
std::string s("Basic");
356+
s.push_back('\0');
357+
s += "evil";
358+
auto r = http_response::unauthorized(std::string_view(s.data(),
359+
s.size()),
360+
"myrealm");
361+
(void)r;
362+
} catch (const std::invalid_argument&) {
363+
caught = true;
364+
}
365+
LT_CHECK_EQ(caught, true);
366+
LT_END_AUTO_TEST(unauthorized_nul_in_scheme_throws)
367+
368+
LT_BEGIN_AUTO_TEST(http_response_factories_suite,
369+
unauthorized_crlf_in_realm_throws)
370+
bool caught = false;
371+
try {
372+
auto r = http_response::unauthorized(
373+
"Basic", "evil\r\nX-Injected: hdr");
374+
(void)r;
375+
} catch (const std::invalid_argument&) {
376+
caught = true;
377+
}
378+
LT_CHECK_EQ(caught, true);
379+
LT_END_AUTO_TEST(unauthorized_crlf_in_realm_throws)
380+
381+
LT_BEGIN_AUTO_TEST(http_response_factories_suite,
382+
unauthorized_lf_in_realm_throws)
383+
bool caught = false;
384+
try {
385+
auto r = http_response::unauthorized("Basic", "evil\nMore: hdr");
386+
(void)r;
387+
} catch (const std::invalid_argument&) {
388+
caught = true;
389+
}
390+
LT_CHECK_EQ(caught, true);
391+
LT_END_AUTO_TEST(unauthorized_lf_in_realm_throws)
392+
393+
LT_BEGIN_AUTO_TEST(http_response_factories_suite,
394+
unauthorized_nul_in_realm_throws)
395+
bool caught = false;
396+
try {
397+
std::string realm("my");
398+
realm.push_back('\0');
399+
realm += "realm";
400+
auto r = http_response::unauthorized(
401+
"Basic", std::string_view(realm.data(), realm.size()));
402+
(void)r;
403+
} catch (const std::invalid_argument&) {
404+
caught = true;
405+
}
406+
LT_CHECK_EQ(caught, true);
407+
LT_END_AUTO_TEST(unauthorized_nul_in_realm_throws)
408+
409+
LT_BEGIN_AUTO_TEST(http_response_factories_suite,
410+
unauthorized_double_quote_in_realm_is_escaped)
411+
// RFC 7235 §2.1: double-quotes inside a quoted-string must be
412+
// backslash-escaped. realm=foo"bar must produce
413+
// WWW-Authenticate: Basic realm="foo\"bar"
414+
auto r = http_response::unauthorized("Basic", R"(foo"bar)");
415+
LT_CHECK_EQ(
416+
r.get_header(httpserver::http::http_utils::http_header_www_authenticate),
417+
std::string(R"(Basic realm="foo\"bar")"));
418+
LT_END_AUTO_TEST(unauthorized_double_quote_in_realm_is_escaped)
419+
304420
// -----------------------------------------------------------------------
305421
// Move smoke: factory results survive being returned from a function.
306422
// Protects against a future regression of the noexcept move ctor.

0 commit comments

Comments
 (0)