mirror of
https://github.com/yhirose/cpp-httplib.git
synced 2026-10-21 11:39:48 +00:00
Apply the request-target check to the client and encode control chars
Share the server's request-target check as fields::is_request_target() and use it in write_request_line too. The client previously used is_field_value(), which let an embedded SP or HTAB through. encode_path() only escaped CR/LF among the control characters, so with path encoding enabled a path like "/a\tb" would now be rejected instead of sent. Percent-encode every control character (0x00-0x1F, 0x7F).
This commit is contained in:
1 parent
e11dbec7b3
commit
c1c2b1f4b4
2 files changed
+59
-26
No files matched your search
@@ -3957,6 +3957,7 @@ bool is_field_vchar(char c);
|
|||||||
bool is_field_content(const std::string &s);
|
bool is_field_content(const std::string &s);
|
||||||
bool is_field_value(const std::string &s);
|
bool is_field_value(const std::string &s);
|
||||||
bool is_field_valid(const std::string &name, const std::string &value);
|
bool is_field_valid(const std::string &name, const std::string &value);
|
||||||
|
bool is_request_target(const std::string &s);
|
||||||
|
|
||||||
} // namespace fields
|
} // namespace fields
|
||||||
} // namespace detail
|
} // namespace detail
|
||||||
@@ -5821,15 +5822,15 @@ inline std::string encode_path(const std::string &s) {
|
|||||||
switch (s[i]) {
|
switch (s[i]) {
|
||||||
case ' ': result += "%20"; break;
|
case ' ': result += "%20"; break;
|
||||||
case '+': result += "%2B"; break;
|
case '+': result += "%2B"; break;
|
||||||
case '\r': result += "%0D"; break;
|
|
||||||
case '\n': result += "%0A"; break;
|
|
||||||
case '\'': result += "%27"; break;
|
case '\'': result += "%27"; break;
|
||||||
case ',': result += "%2C"; break;
|
case ',': result += "%2C"; break;
|
||||||
// case ':': result += "%3A"; break; // ok? probably...
|
// case ':': result += "%3A"; break; // ok? probably...
|
||||||
case ';': result += "%3B"; break;
|
case ';': result += "%3B"; break;
|
||||||
default:
|
default:
|
||||||
auto c = static_cast<uint8_t>(s[i]);
|
auto c = static_cast<uint8_t>(s[i]);
|
||||||
if (c >= 0x80) {
|
// Control characters (incl. CR/LF) and non-ASCII bytes are not allowed
|
||||||
|
// in a request-target as-is.
|
||||||
|
if (c < 0x20 || c == 0x7f || c >= 0x80) {
|
||||||
result += '%';
|
result += '%';
|
||||||
char hex[4];
|
char hex[4];
|
||||||
auto len = snprintf(hex, sizeof(hex) - 1, "%02X", c);
|
auto len = snprintf(hex, sizeof(hex) - 1, "%02X", c);
|
||||||
@@ -8561,14 +8562,11 @@ bool read_content(Stream &strm, T &x, size_t payload_max_length, int &status,
|
|||||||
|
|
||||||
inline ssize_t write_request_line(Stream &strm, const std::string &method,
|
inline ssize_t write_request_line(Stream &strm, const std::string &method,
|
||||||
const std::string &path) {
|
const std::string &path) {
|
||||||
// Neither the method nor the request target may carry CR/LF (or other
|
// Neither the method nor the request target may carry CR/LF, SP or other
|
||||||
// control octets); otherwise a value smuggled into either splits the request
|
// control octets; otherwise a value smuggled into either splits the request
|
||||||
// line and injects headers or a whole request. The method must be a token
|
// line and injects headers or a whole request.
|
||||||
// (RFC 9110 Section 9.1), which also rejects an empty method and embedded
|
|
||||||
// spaces. The target gets the same field-value check that already guards
|
|
||||||
// header values in check_and_write_headers.
|
|
||||||
if (!fields::is_token(method)) { return -1; }
|
if (!fields::is_token(method)) { return -1; }
|
||||||
if (!fields::is_field_value(path)) { return -1; }
|
if (!fields::is_request_target(path)) { return -1; }
|
||||||
|
|
||||||
std::string s = method;
|
std::string s = method;
|
||||||
s += ' ';
|
s += ' ';
|
||||||
@@ -10222,6 +10220,12 @@ inline bool is_field_valid(const std::string &name, const std::string &value) {
|
|||||||
return is_field_name(name) && is_field_value(value);
|
return is_field_name(name) && is_field_value(value);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// RFC 9112 §2.2/§3.2: the request-target has no SP, HTAB or other control
|
||||||
|
// characters (incl. bare CR). obs-text (raw UTF-8) is allowed.
|
||||||
|
inline bool is_request_target(const std::string &s) {
|
||||||
|
return std::all_of(s.begin(), s.end(), is_field_vchar);
|
||||||
|
}
|
||||||
|
|
||||||
} // namespace fields
|
} // namespace fields
|
||||||
|
|
||||||
inline bool perform_websocket_handshake(Stream &strm, Request &req,
|
inline bool perform_websocket_handshake(Stream &strm, Request &req,
|
||||||
@@ -13296,12 +13300,7 @@ inline bool Server::parse_request_line(const char *s, Request &req) const {
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
// RFC 9112 §2.2/§3.2: reject control characters (incl. bare CR) in the
|
if (!detail::fields::is_request_target(req.target)) { return false; }
|
||||||
// request-target. obs-text is allowed since some clients send raw UTF-8.
|
|
||||||
if (!std::all_of(req.target.begin(), req.target.end(),
|
|
||||||
detail::fields::is_field_vchar)) {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
{
|
{
|
||||||
// Skip URL fragment
|
// Skip URL fragment
|
||||||
|
|||||||
+44
-10
@@ -4060,6 +4060,37 @@ TEST(PathUrlEncodeTest, StreamingCRLFInTargetIsEncoded) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
TEST(PathUrlEncodeTest, ControlCharsInPathAreEncoded) {
|
||||||
|
// Every control character is percent-encoded, not just CR/LF, so the target
|
||||||
|
// passes the request-target check in write_request_line.
|
||||||
|
Server svr;
|
||||||
|
|
||||||
|
std::string target;
|
||||||
|
svr.set_pre_routing_handler([&](const Request &req, Response &res) {
|
||||||
|
target = req.target;
|
||||||
|
res.status = StatusCode::OK_200;
|
||||||
|
return Server::HandlerResponse::Handled;
|
||||||
|
});
|
||||||
|
|
||||||
|
auto port = svr.bind_to_any_port(HOST);
|
||||||
|
auto thread = std::thread([&]() { svr.listen_after_bind(); });
|
||||||
|
auto se = detail::scope_exit([&] {
|
||||||
|
svr.stop();
|
||||||
|
thread.join();
|
||||||
|
ASSERT_FALSE(svr.is_running());
|
||||||
|
});
|
||||||
|
|
||||||
|
svr.wait_until_ready();
|
||||||
|
|
||||||
|
{
|
||||||
|
Client cli(HOST, port);
|
||||||
|
|
||||||
|
auto res = cli.Get("/a\tb\x1b\x7f");
|
||||||
|
ASSERT_TRUE(res) << "Error: " << to_string(res.error());
|
||||||
|
EXPECT_EQ("/a%09b%1B%7F", target);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
TEST(PathUrlEncodeTest, StreamingCRLFRejectedWhenPathEncodeDisabled) {
|
TEST(PathUrlEncodeTest, StreamingCRLFRejectedWhenPathEncodeDisabled) {
|
||||||
// Nothing may reach the wire: a raw CR/LF target would split the request
|
// Nothing may reach the wire: a raw CR/LF target would split the request
|
||||||
// line and inject headers.
|
// line and inject headers.
|
||||||
@@ -10092,7 +10123,7 @@ ssize_t write_request_line(Stream &strm, const std::string &method,
|
|||||||
} // namespace detail
|
} // namespace detail
|
||||||
} // namespace httplib
|
} // namespace httplib
|
||||||
|
|
||||||
TEST(RequestLineInjectionTest, RejectsCRLFInTarget) {
|
TEST(RequestLineInjectionTest, RejectsInvalidCharsInTarget) {
|
||||||
// A well-formed target is written verbatim.
|
// A well-formed target is written verbatim.
|
||||||
{
|
{
|
||||||
detail::BufferStream strm;
|
detail::BufferStream strm;
|
||||||
@@ -10101,15 +10132,18 @@ TEST(RequestLineInjectionTest, RejectsCRLFInTarget) {
|
|||||||
EXPECT_EQ("GET /path?a=b HTTP/1.1\r\n", strm.get_buffer());
|
EXPECT_EQ("GET /path?a=b HTTP/1.1\r\n", strm.get_buffer());
|
||||||
}
|
}
|
||||||
|
|
||||||
// A target carrying CR/LF must be rejected before anything reaches the wire,
|
// A target carrying CR/LF, SP or other control octets must be rejected
|
||||||
// otherwise it splits the request line and injects a header or a whole
|
// before anything reaches the wire, otherwise it splits the request line and
|
||||||
// request. This is what a decoded redirect Location ("%0D%0A") turns into
|
// injects a header or a whole request. This is what a decoded redirect
|
||||||
// when path encoding is disabled.
|
// Location ("%0D%0A") turns into when path encoding is disabled.
|
||||||
const std::string evil_targets[] = {
|
const std::string evil_targets[] = {
|
||||||
"/a\r\nInjected: pwned",
|
"/a\r\nInjected: pwned",
|
||||||
"/a\rInjected",
|
"/a\rInjected",
|
||||||
"/a\nInjected",
|
"/a\nInjected",
|
||||||
"/a\r\n\r\nGET /evil HTTP/1.1\r\nHost: victim\r\n\r\n",
|
"/a\r\n\r\nGET /evil HTTP/1.1\r\nHost: victim\r\n\r\n",
|
||||||
|
"/a b",
|
||||||
|
"/a\tb",
|
||||||
|
"/a\x7f",
|
||||||
};
|
};
|
||||||
for (const auto &evil : evil_targets) {
|
for (const auto &evil : evil_targets) {
|
||||||
detail::BufferStream strm;
|
detail::BufferStream strm;
|
||||||
@@ -10120,11 +10154,11 @@ TEST(RequestLineInjectionTest, RejectsCRLFInTarget) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
TEST(RequestLineInjectionTest, ClientRejectsCRLFTargetEndToEnd) {
|
TEST(RequestLineInjectionTest, ClientRejectsCRLFTargetEndToEnd) {
|
||||||
// End-to-end counterpart to RejectsCRLFInTarget. With path encoding disabled
|
// End-to-end counterpart to RejectsInvalidCharsInTarget. With path encoding
|
||||||
// the client transmits the target verbatim, so a CR/LF-bearing target -- what
|
// disabled the client transmits the target verbatim, so a CR/LF-bearing
|
||||||
// a redirect Location "%0D%0A" decodes to -- reaches write_request. The
|
// target -- what a redirect Location "%0D%0A" decodes to -- reaches
|
||||||
// client must fail cleanly with Error::Write instead of putting a
|
// write_request. The client must fail cleanly with Error::Write instead of
|
||||||
// request-line-less, header-injecting request on the wire.
|
// putting a request-line-less, header-injecting request on the wire.
|
||||||
Server svr;
|
Server svr;
|
||||||
|
|
||||||
svr.Get("/a", [](const Request &, Response &res) {
|
svr.Get("/a", [](const Request &, Response &res) {
|
||||||
|
|||||||
Reference in new issue
Block a user