Skip to content

Commit a3e53f3

Browse files
etrclaude
andcommitted
security: fix auth bypass via path-normalization mismatch (dot-segments)
The v2.0 auth pipeline interpreted the request path two different ways that disagreed on ".." handling, letting an attacker reach a protected handler with authentication skipped. should_skip_auth() ran the path through normalize_path(), which collapses "." / ".." segments, then matched it against auth_skip_paths. But the route matcher (radix_tree::find via mr->standardized_url) and the regex tier saw the raw path -- standardize_url() only collapses duplicate '/' and a trailing '/', never dot-segments. On a failed descent the radix walk falls back to the deepest matched prefix terminus. So with a global auth_handler, a protected register_prefix("/admin") (or "/admin/.*" regex) route, and auth_skip_paths({"/public/*"}): GET /admin/../public/x normalized to "/public/x" for the auth-skip check (auth skipped) yet still routed to the protected "/admin" prefix handler -- served unauthenticated. (Requires a client that does not pre-normalize "..", e.g. curl --path-as-is; MHD does not collapse dot-segments.) Fix: canonicalize once, at the single point in answer_to_connection where the routing/auth path is derived, by applying normalize_path() to the standardized URL. The router, should_skip_auth(), and the route-cache key now all interpret the path identically; the request above is uniformly treated as "/public/x" and never reaches /admin. The extra normalize_path in should_skip_auth() is now idempotent. Adds an end-to-end regression test (auth_skip_dotdot_no_route_confusion) that sends "/admin/../public/x" with CURLOPT_PATH_AS_IS and asserts the protected handler is never reached; it fails on the pre-fix code with ADMIN_SECRET != PUBLIC_OK. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpysYDDJac63yz2mZKKiDf
1 parent 7ee0463 commit a3e53f3

2 files changed

Lines changed: 79 additions & 1 deletion

File tree

src/detail/webserver_request.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,20 @@ MHD_Result webserver_impl::answer_to_connection(void* cls, MHD_Connection* conne
417417

418418
std::string t_url = url;
419419
base_unescaper(&t_url, impl->parent->unescaper);
420-
mr->standardized_url = http_utils::standardize_url(t_url);
420+
// SECURITY: collapse dot-segments ("." / "..") into the canonical
421+
// path here, at the single point where the routing/auth path is
422+
// derived. Both the route matcher (radix_tree::find via
423+
// mr->standardized_url) and should_skip_auth() must interpret the
424+
// path identically; should_skip_auth() runs the path through
425+
// normalize_path() (which pops ".."), but standardize_url() only
426+
// collapses duplicate '/' and a trailing '/'. Without this, a request
427+
// such as "/admin/../public/x" normalizes to "/public/x" for the
428+
// auth-skip check (auth skipped) yet the router still descends to the
429+
// "/admin" prefix/regex handler -- an authentication bypass. Applying
430+
// normalize_path() to the standardized URL makes the two views agree;
431+
// it is idempotent w.r.t. the normalize_path() call already in
432+
// should_skip_auth().
433+
mr->standardized_url = normalize_path(http_utils::standardize_url(t_url));
421434
mr->has_body = false;
422435

423436
// log_access is now a response_sent alias (see webserver_aliases.cpp).

test/integ/authentication.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,71 @@ LT_BEGIN_AUTO_TEST(authentication_suite, auth_skip_paths_deep_nested)
972972
ws.stop();
973973
LT_END_AUTO_TEST(auth_skip_paths_deep_nested)
974974

975+
// SECURITY REGRESSION (path-normalization auth bypass): the auth-skip
976+
// check and the route matcher must interpret the request path
977+
// identically. Before the fix, should_skip_auth() collapsed ".."
978+
// segments while the router did not, so "GET /admin/../public/x" looked
979+
// like "/public/x" to the auth gate (auth skipped) yet still descended
980+
// to the protected "/admin" prefix handler -- serving it unauthenticated.
981+
// After the fix the dispatch path canonicalizes the URL once (dot-
982+
// segments collapsed at answer_to_connection), so the same request is
983+
// uniformly treated as "/public/x": auth is legitimately skipped and it
984+
// routes to the public handler, never to /admin.
985+
//
986+
// CURLOPT_PATH_AS_IS is required -- libcurl collapses "/../" client-side
987+
// by default, which would mask the server-side behaviour under test.
988+
class admin_secret_resource : public http_resource {
989+
public:
990+
http_response render_get(const http_request&) {
991+
return http_response::string("ADMIN_SECRET");
992+
}
993+
};
994+
995+
class public_ok_resource : public http_resource {
996+
public:
997+
http_response render_get(const http_request&) {
998+
return http_response::string("PUBLIC_OK");
999+
}
1000+
};
1001+
1002+
LT_BEGIN_AUTO_TEST(authentication_suite, auth_skip_dotdot_no_route_confusion)
1003+
webserver ws{create_webserver(0)
1004+
.auth_handler(centralized_auth_handler)
1005+
.auth_skip_paths({"/public/*"})};
1006+
1007+
ws.register_prefix("admin", std::make_shared<admin_secret_resource>());
1008+
ws.register_prefix("public", std::make_shared<public_ok_resource>());
1009+
ws.start(false);
1010+
const uint16_t port = ws.get_bound_port();
1011+
1012+
curl_global_init(CURL_GLOBAL_ALL);
1013+
std::string s;
1014+
CURL *curl = curl_easy_init();
1015+
CURLcode res;
1016+
long http_code = 0; // NOLINT(runtime/int)
1017+
1018+
// Attacker path: normalizes to /public/x for the auth-skip check,
1019+
// but a pre-fix router would descend to the /admin prefix handler.
1020+
const std::string url =
1021+
"localhost:" + std::to_string(port) + "/admin/../public/x";
1022+
curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
1023+
curl_easy_setopt(curl, CURLOPT_PATH_AS_IS, 1L);
1024+
curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L);
1025+
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writefunc);
1026+
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &s);
1027+
res = curl_easy_perform(curl);
1028+
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
1029+
LT_ASSERT_EQ(res, 0);
1030+
// Must NOT reach the protected /admin handler unauthenticated.
1031+
LT_CHECK(s != "ADMIN_SECRET");
1032+
// Uniformly canonicalized to /public/x -> served by the public handler.
1033+
LT_CHECK_EQ(http_code, 200);
1034+
LT_CHECK_EQ(s, "PUBLIC_OK");
1035+
curl_easy_cleanup(curl);
1036+
1037+
ws.stop();
1038+
LT_END_AUTO_TEST(auth_skip_dotdot_no_route_confusion)
1039+
9751040
// Test POST method with centralized auth
9761041
class post_resource : public http_resource {
9771042
public:

0 commit comments

Comments
 (0)