Skip to content

Commit bb15591

Browse files
committed
Fix header parser
1 parent e9eb752 commit bb15591

File tree

3 files changed

+46
-23
lines changed

3 files changed

+46
-23
lines changed

CHANGELOG

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Release 6.0.26 (Not yet released)
22
-------------
3-
*
3+
* [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.
44

55

66
Release 6.0.25

src/cxx_supportlib/ServerKit/HttpHeaderParser.h

+19-21
Original file line numberDiff line numberDiff line change
@@ -119,31 +119,26 @@ class HttpHeaderParser {
119119
}
120120

121121
static size_t http_parser_execute_and_handle_pause(llhttp_t *parser,
122-
const char *data, size_t len, bool &paused)
122+
const char *data, size_t len)
123123
{
124124
llhttp_errno_t rc = llhttp_get_errno(parser);
125125
switch (rc) {
126126
case HPE_PAUSED_UPGRADE:
127127
llhttp_resume_after_upgrade(parser);
128+
rc = llhttp_get_errno(parser);
128129
goto happy_path;
129130
case HPE_PAUSED:
130131
llhttp_resume(parser);
132+
rc = llhttp_get_errno(parser);
131133
goto happy_path;
132134
case HPE_OK:
135+
rc = llhttp_execute(parser, data, len);
133136
happy_path:
134-
switch (llhttp_execute(parser, data, len)) {
135-
case HPE_PAUSED_H2_UPGRADE:
136-
case HPE_PAUSED_UPGRADE:
137-
case HPE_PAUSED:
138-
paused = true;
139-
return (llhttp_get_error_pos(parser) - data);
140-
case HPE_OK:
137+
if (rc == HPE_OK) {
141138
return len;
142-
default:
143-
goto error_path;
144-
}
139+
}
140+
// deliberate fall through
145141
default:
146-
error_path:
147142
return (llhttp_get_error_pos(parser) - data);
148143
}
149144
}
@@ -488,20 +483,22 @@ class HttpHeaderParser {
488483
TRACE_POINT();
489484
P_ASSERT_EQ(message->httpState, Message::PARSING_HEADERS);
490485

491-
size_t ret;
492-
bool paused;
493-
494486
state->parser.data = this;
495487
currentBuffer = &buffer;
496-
ret = http_parser_execute_and_handle_pause(&state->parser,
497-
buffer.start, buffer.size(), paused);
488+
size_t ret = http_parser_execute_and_handle_pause(&state->parser,
489+
buffer.start, buffer.size());
498490
currentBuffer = NULL;
499491

500-
if (!llhttp_get_upgrade(&state->parser) && ret != buffer.size() && !paused || !paused && llhttp_get_errno(&state->parser) != HPE_OK) {
492+
llhttp_errno_t llerrno = llhttp_get_errno(&state->parser);
493+
494+
bool paused = (llerrno == HPE_PAUSED_H2_UPGRADE || llerrno == HPE_PAUSED_UPGRADE || llerrno == HPE_PAUSED);
495+
496+
if ( (!llhttp_get_upgrade(&state->parser) && ret != buffer.size() && !paused) ||
497+
(llerrno != HPE_OK && !paused) ) {
501498
UPDATE_TRACE_POINT();
502499
message->httpState = Message::ERROR;
503-
switch (llhttp_get_errno(&state->parser)) {
504-
case HPE_CB_HEADER_FIELD_COMPLETE://?? does this match was HPE_CB_header_field in old one
500+
switch (llerrno) {
501+
case HPE_CB_HEADER_FIELD_COMPLETE:// does this match? was HPE_CB_header_field in old impl
505502
case HPE_CB_HEADERS_COMPLETE:
506503
switch (state->state) {
507504
case HttpHeaderParserState::ERROR_SECURITY_PASSWORD_MISMATCH:
@@ -526,9 +523,10 @@ class HttpHeaderParser {
526523
break;
527524
default:
528525
default_error:
529-
message->aux.parseError = HTTP_PARSER_ERRNO_BEGIN - llhttp_get_errno(&state->parser);
526+
message->aux.parseError = HTTP_PARSER_ERRNO_BEGIN - llerrno;
530527
break;
531528
}
529+
llhttp_finish(&state->parser);
532530
} else if (messageHttpStateIndicatesCompletion(MessageType())) {
533531
UPDATE_TRACE_POINT();
534532
message->httpMajor = llhttp_get_http_major(&state->parser);

test/cxx/ServerKit/HttpServerTest.cpp

+26-1
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,32 @@ namespace tut {
806806
"hello /");
807807
}
808808

809+
TEST_METHOD(19) {
810+
set_test_name("It responds with correct error if http method is not recognized");
811+
812+
// send invalid request
813+
connectToServer();
814+
sendRequest("BAD_METHOD / HTTP/1.1\r\n"
815+
"Connection: close\r\n"
816+
"Host: foo\r\n\r\n");
817+
string response = readAll(fd, 1024).first;
818+
819+
ensure("Response starts with error",
820+
startsWith(response,
821+
"HTTP/1.0 400 Bad Request\r\n"
822+
"Status: 400 Bad Request\r\n"
823+
"Content-Type: text/html; charset=UTF-8\r\n"));
824+
825+
ensure("Response ends with error",
826+
endsWith(response,
827+
"Connection: close\r\n"
828+
"Content-Length: 19\r\n"
829+
"cache-control: no-cache, no-store, must-revalidate\r\n"
830+
"\r\n"
831+
"invalid HTTP method"));
832+
ensure_equals("Response size is correct", response.size(), 242u);
833+
}
834+
809835
/***** Fixed body handling *****/
810836

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

1480-
14811506
/***** Secure headers handling *****/
14821507

14831508
TEST_METHOD(60) {

0 commit comments

Comments
 (0)