Skip to content

Commit

Permalink
Fix header parser
Browse files Browse the repository at this point in the history
  • Loading branch information
CamJN committed Feb 18, 2025
1 parent e9eb752 commit bb15591
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 23 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Release 6.0.26 (Not yet released)
-------------
*
* [CVE-2025-26803] The http parser (from Passenger 6.0.21-6.0.25) was susceptible to a denial of service attack when parsing a request with an invalid HTTP method.


Release 6.0.25
Expand Down
40 changes: 19 additions & 21 deletions src/cxx_supportlib/ServerKit/HttpHeaderParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,31 +119,26 @@ class HttpHeaderParser {
}

static size_t http_parser_execute_and_handle_pause(llhttp_t *parser,
const char *data, size_t len, bool &paused)
const char *data, size_t len)
{
llhttp_errno_t rc = llhttp_get_errno(parser);
switch (rc) {
case HPE_PAUSED_UPGRADE:
llhttp_resume_after_upgrade(parser);
rc = llhttp_get_errno(parser);
goto happy_path;
case HPE_PAUSED:
llhttp_resume(parser);
rc = llhttp_get_errno(parser);
goto happy_path;
case HPE_OK:
rc = llhttp_execute(parser, data, len);
happy_path:
switch (llhttp_execute(parser, data, len)) {
case HPE_PAUSED_H2_UPGRADE:
case HPE_PAUSED_UPGRADE:
case HPE_PAUSED:
paused = true;
return (llhttp_get_error_pos(parser) - data);
case HPE_OK:
if (rc == HPE_OK) {
return len;
default:
goto error_path;
}
}
// deliberate fall through
default:
error_path:
return (llhttp_get_error_pos(parser) - data);
}
}
Expand Down Expand Up @@ -488,20 +483,22 @@ class HttpHeaderParser {
TRACE_POINT();
P_ASSERT_EQ(message->httpState, Message::PARSING_HEADERS);

size_t ret;
bool paused;

state->parser.data = this;
currentBuffer = &buffer;
ret = http_parser_execute_and_handle_pause(&state->parser,
buffer.start, buffer.size(), paused);
size_t ret = http_parser_execute_and_handle_pause(&state->parser,
buffer.start, buffer.size());
currentBuffer = NULL;

if (!llhttp_get_upgrade(&state->parser) && ret != buffer.size() && !paused || !paused && llhttp_get_errno(&state->parser) != HPE_OK) {
llhttp_errno_t llerrno = llhttp_get_errno(&state->parser);

bool paused = (llerrno == HPE_PAUSED_H2_UPGRADE || llerrno == HPE_PAUSED_UPGRADE || llerrno == HPE_PAUSED);

if ( (!llhttp_get_upgrade(&state->parser) && ret != buffer.size() && !paused) ||
(llerrno != HPE_OK && !paused) ) {
UPDATE_TRACE_POINT();
message->httpState = Message::ERROR;
switch (llhttp_get_errno(&state->parser)) {
case HPE_CB_HEADER_FIELD_COMPLETE://?? does this match was HPE_CB_header_field in old one
switch (llerrno) {
case HPE_CB_HEADER_FIELD_COMPLETE:// does this match? was HPE_CB_header_field in old impl
case HPE_CB_HEADERS_COMPLETE:
switch (state->state) {
case HttpHeaderParserState::ERROR_SECURITY_PASSWORD_MISMATCH:
Expand All @@ -526,9 +523,10 @@ class HttpHeaderParser {
break;
default:
default_error:
message->aux.parseError = HTTP_PARSER_ERRNO_BEGIN - llhttp_get_errno(&state->parser);
message->aux.parseError = HTTP_PARSER_ERRNO_BEGIN - llerrno;
break;
}
llhttp_finish(&state->parser);
} else if (messageHttpStateIndicatesCompletion(MessageType())) {
UPDATE_TRACE_POINT();
message->httpMajor = llhttp_get_http_major(&state->parser);
Expand Down
27 changes: 26 additions & 1 deletion test/cxx/ServerKit/HttpServerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,32 @@ namespace tut {
"hello /");
}

TEST_METHOD(19) {
set_test_name("It responds with correct error if http method is not recognized");

// send invalid request
connectToServer();
sendRequest("BAD_METHOD / HTTP/1.1\r\n"
"Connection: close\r\n"
"Host: foo\r\n\r\n");
string response = readAll(fd, 1024).first;

ensure("Response starts with error",
startsWith(response,
"HTTP/1.0 400 Bad Request\r\n"
"Status: 400 Bad Request\r\n"
"Content-Type: text/html; charset=UTF-8\r\n"));

ensure("Response ends with error",
endsWith(response,
"Connection: close\r\n"
"Content-Length: 19\r\n"
"cache-control: no-cache, no-store, must-revalidate\r\n"
"\r\n"
"invalid HTTP method"));
ensure_equals("Response size is correct", response.size(), 242u);
}

/***** Fixed body handling *****/

TEST_METHOD(20) {
Expand Down Expand Up @@ -1477,7 +1503,6 @@ namespace tut {
ensure("(1)", containsSubstring(response, "HTTP/1.1 400 Bad Request\r\n"));
}


/***** Secure headers handling *****/

TEST_METHOD(60) {
Expand Down

0 comments on commit bb15591

Please sign in to comment.