Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lenient flag to make CR completely optional. #243

Merged
merged 2 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,14 @@ Normally `llhttp` would error when a CR is not followed by LF when terminating t
request line, the status line, the headers or a chunk header.
With this flag only a CR is required to terminate such sections.

### `void llhttp_set_lenient_optional_cr_before_lf(llhttp_t* parser, int enabled)`

Enables/disables lenient handling of line separators.

Normally `llhttp` would error when a LF is not preceded by CR when terminating the
request line, the status line, the headers, a chunk header or a chunk data.
With this flag only a LF is required to terminate such sections.

**Enabling this flag can pose a security issue since you will be exposed to request smuggling attacks. USE WITH CAUTION!**

### `void llhttp_set_lenient_optional_crlf_after_chunk(llhttp_t* parser, int enabled)`
Expand Down
1 change: 1 addition & 0 deletions src/llhttp/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export enum LENIENT_FLAGS {
DATA_AFTER_CLOSE = 1 << 5,
OPTIONAL_LF_AFTER_CR = 1 << 6,
OPTIONAL_CRLF_AFTER_CHUNK = 1 << 7,
OPTIONAL_CR_BEFORE_LF = 1 << 8,
}

export enum METHODS {
Expand Down
191 changes: 131 additions & 60 deletions src/llhttp/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export class HTTP {
p.property('i8', 'http_major');
p.property('i8', 'http_minor');
p.property('i8', 'header_state');
p.property('i8', 'lenient_flags');
p.property('i16', 'lenient_flags');
p.property('i8', 'upgrade');
p.property('i8', 'finish');
p.property('i16', 'flags');
Expand Down Expand Up @@ -311,6 +311,10 @@ export class HTTP {
);
};

const checkIfAllowLFWithoutCR = (success: Node, failure: Node) => {
return this.testLenientFlags(LENIENT_FLAGS.OPTIONAL_CR_BEFORE_LF, { 1: success }, failure);
};

// Response
n('start_res')
.match('HTTP/', span.version.start(n('res_http_major')))
Expand All @@ -329,7 +333,7 @@ export class HTTP {
.otherwise(this.span.version.end(p.error(ERROR.INVALID_VERSION, 'Invalid minor version')));

n('res_http_end')
.otherwise(this.span.version.end().otherwise(
.otherwise(this.span.version.end(
this.invokePausable('on_version_complete', ERROR.CB_VERSION_COMPLETE, 'res_after_version'),
));

Expand Down Expand Up @@ -359,23 +363,36 @@ export class HTTP {
}))
.otherwise(p.error(ERROR.INVALID_STATUS, 'Invalid status code'));

n('res_status_code_otherwise')
.match(' ', n('res_status_start'))
.peek([ '\r', '\n' ], n('res_status_start'))
.otherwise(p.error(ERROR.INVALID_STATUS, 'Invalid response status'));

const onStatusComplete = this.invokePausable(
'on_status_complete', ERROR.CB_STATUS_COMPLETE, n('headers_start'),
);

n('res_status_start')
n('res_status_code_otherwise')
.match(' ', n('res_status_start'))
.match('\r', n('res_line_almost_done'))
.match('\n', onStatusComplete)
.match(
'\n',
checkIfAllowLFWithoutCR(
onStatusComplete,
p.error(ERROR.INVALID_STATUS, 'Invalid response status'),
),
)
.otherwise(p.error(ERROR.INVALID_STATUS, 'Invalid response status'));

n('res_status_start')
.otherwise(span.status.start(n('res_status')));

n('res_status')
.peek('\r', span.status.end().skipTo(n('res_line_almost_done')))
.peek('\n', span.status.end().skipTo(n('res_line_almost_done')))
.peek(
'\n',
span.status.end().skipTo(
checkIfAllowLFWithoutCR(
onStatusComplete,
p.error(ERROR.CR_EXPECTED, 'Missing expected CR after response line'),
),
),
)
.skipTo(n('res_status'));

n('res_line_almost_done')
Expand Down Expand Up @@ -458,18 +475,26 @@ export class HTTP {
.otherwise(this.span.version.end(p.error(ERROR.INVALID_VERSION, 'Invalid minor version')));

n('req_http_end').otherwise(
span.version.end().otherwise(
span.version.end(
this.invokePausable(
'on_version_complete',
ERROR.CB_VERSION_COMPLETE,
this.load('method', {
[METHODS.PRI]: n('req_pri_upgrade'),
}, n('req_http_complete')),
)),
),
),
);

n('req_http_complete')
.match('\r', n('req_http_complete_crlf'))
.match(
'\n',
checkIfAllowLFWithoutCR(
n('req_http_complete_crlf'),
p.error(ERROR.INVALID_VERSION, 'Expected CRLF after version'),
),
)
.otherwise(p.error(ERROR.INVALID_VERSION, 'Expected CRLF after version'));

n('req_http_complete_crlf')
Expand Down Expand Up @@ -509,8 +534,11 @@ export class HTTP {
n('header_field_start')
.match('\r', n('headers_almost_done'))
.match('\n',
this.testLenientFlags(LENIENT_FLAGS.HEADERS, {
1: n('headers_done'),
this.testLenientFlags(LENIENT_FLAGS.OPTIONAL_CR_BEFORE_LF, {
1: this.testFlags(FLAGS.TRAILING, {
1: this.invokePausable('on_chunk_complete',
ERROR.CB_CHUNK_COMPLETE, 'message_done'),
}).otherwise(n('headers_done')),
}, onInvalidHeaderFieldChar),
)
.otherwise(span.headerField.start(n('header_field')));
Expand All @@ -521,8 +549,40 @@ export class HTTP {
.select(SPECIAL_HEADERS, this.store('header_state', 'header_field_colon'))
.otherwise(this.resetHeaderState('header_field_general'));

/* https://www.rfc-editor.org/rfc/rfc7230.html#section-3.3.3, paragraph 3.
*
* If a message is received with both a Transfer-Encoding and a
* Content-Length header field, the Transfer-Encoding overrides the
* Content-Length. Such a message might indicate an attempt to
* perform request smuggling (Section 9.5) or response splitting
* (Section 9.4) and **ought to be handled as an error**. A sender MUST
* remove the received Content-Length field prior to forwarding such
* a message downstream.
*
* Since llhttp 9, we go for the stricter approach and treat this as an error.
*/
const checkInvalidTransferEncoding = (otherwise: Node) => {
return this.testFlags(FLAGS.CONTENT_LENGTH, {
1: this.testLenientFlags(LENIENT_FLAGS.CHUNKED_LENGTH, {
0: p.error(ERROR.INVALID_TRANSFER_ENCODING, "Transfer-Encoding can't be present with Content-Length"),
}).otherwise(otherwise),
}).otherwise(otherwise);
};

const checkInvalidContentLength = (otherwise: Node) => {
return this.testFlags(FLAGS.TRANSFER_ENCODING, {
1: this.testLenientFlags(LENIENT_FLAGS.CHUNKED_LENGTH, {
0: p.error(ERROR.INVALID_CONTENT_LENGTH, "Content-Length can't be present with Transfer-Encoding"),
}).otherwise(otherwise),
}).otherwise(otherwise);
};

const onHeaderFieldComplete = this.invokePausable(
'on_header_field_complete', ERROR.CB_HEADER_FIELD_COMPLETE, n('header_value_discard_ws'),
'on_header_field_complete', ERROR.CB_HEADER_FIELD_COMPLETE,
this.load('header_state', {
[HEADER_STATE.TRANSFER_ENCODING]: checkInvalidTransferEncoding(n('header_value_discard_ws')),
[HEADER_STATE.CONTENT_LENGTH]: checkInvalidContentLength(n('header_value_discard_ws')),
}, 'header_value_discard_ws'),
);

const checkLenientFlagsOnColon =
Expand Down Expand Up @@ -570,7 +630,7 @@ export class HTTP {
n('header_value_discard_ws')
.match([ ' ', '\t' ], n('header_value_discard_ws'))
.match('\r', n('header_value_discard_ws_almost_done'))
.match('\n', this.testLenientFlags(LENIENT_FLAGS.HEADERS, {
.match('\n', this.testLenientFlags(LENIENT_FLAGS.OPTIONAL_CR_BEFORE_LF, {
1: n('header_value_discard_lws'),
}, p.error(ERROR.INVALID_HEADER_TOKEN, 'Invalid header value char')))
.otherwise(span.headerValue.start(n('header_value_start')));
Expand Down Expand Up @@ -734,25 +794,32 @@ export class HTTP {
.match(HEADER_CHARS, n('header_value'))
.otherwise(n('header_value_otherwise'));

const checkIfAllowLFWithoutCR = (success: Node, failure: Node) => {
return this.testLenientFlags(LENIENT_FLAGS.OPTIONAL_CR_BEFORE_LF, { 1: success }, failure);
};

const checkLenient = this.testLenientFlags(LENIENT_FLAGS.HEADERS, {
1: n('header_value_lenient'),
}, span.headerValue.end(p.error(ERROR.INVALID_HEADER_TOKEN, 'Invalid header value char')));

n('header_value_otherwise')
.peek('\r', span.headerValue.end().skipTo(n('header_value_almost_done')))
.peek(
'\n',
span.headerValue.end(
checkIfAllowLFWithoutCR(
n('header_value_almost_done'),
p.error(ERROR.CR_EXPECTED, 'Missing expected CR after header value'),
),
),
)
.otherwise(checkLenient);

n('header_value_lenient')
.peek('\r', span.headerValue.end().skipTo(n('header_value_almost_done')))
.peek('\n', span.headerValue.end(n('header_value_almost_done')))
.skipTo(n('header_value_lenient'));

n('header_value_lenient_failed')
.peek('\n', span.headerValue.end().skipTo(
p.error(ERROR.CR_EXPECTED, 'Missing expected CR after header value')),
)
.otherwise(p.error(ERROR.INVALID_HEADER_TOKEN, 'Invalid header value char'));

n('header_value_almost_done')
.match('\n', n('header_value_lws'))
.otherwise(p.error(ERROR.LF_EXPECTED,
Expand All @@ -766,10 +833,13 @@ export class HTTP {
}, span.headerValue.start(n('header_value_start'))))
.otherwise(this.setHeaderFlags(onHeaderValueComplete));

// Set `upgrade` if needed
const beforeHeadersComplete = p.invoke(callback.beforeHeadersComplete);

const checkTrailing = this.testFlags(FLAGS.TRAILING, {
1: this.invokePausable('on_chunk_complete',
ERROR.CB_CHUNK_COMPLETE, 'message_done'),
});
}).otherwise(beforeHeadersComplete);

n('headers_almost_done')
.match('\n', checkTrailing)
Expand All @@ -778,43 +848,7 @@ export class HTTP {
1: checkTrailing,
}, p.error(ERROR.STRICT, 'Expected LF after headers')));

// Set `upgrade` if needed
const beforeHeadersComplete = p.invoke(callback.beforeHeadersComplete);

/* Present `Transfer-Encoding` header overrides `Content-Length` even if the
* actual coding is not `chunked`. As per spec:
*
* https://www.rfc-editor.org/rfc/rfc7230.html#section-3.3.3
*
* If a message is received with both a Transfer-Encoding and a
* Content-Length header field, the Transfer-Encoding overrides the
* Content-Length. Such a message might indicate an attempt to
* perform request smuggling (Section 9.5) or response splitting
* (Section 9.4) and **ought to be handled as an error**. A sender MUST
* remove the received Content-Length field prior to forwarding such
* a message downstream.
*
* (Note our emphasis on **ought to be handled as an error**
*/

const ENCODING_CONFLICT = FLAGS.TRANSFER_ENCODING | FLAGS.CONTENT_LENGTH;

const onEncodingConflict =
this.testLenientFlags(LENIENT_FLAGS.CHUNKED_LENGTH, {
0: p.error(ERROR.UNEXPECTED_CONTENT_LENGTH,
'Content-Length can\'t be present with Transfer-Encoding'),

// For LENIENT mode fall back to past behavior:
// Ignore `Transfer-Encoding` when `Content-Length` is present.
}).otherwise(beforeHeadersComplete);

const checkEncConflict = this.testFlags(ENCODING_CONFLICT, {
1: onEncodingConflict,
}).otherwise(beforeHeadersComplete);

checkTrailing.otherwise(checkEncConflict);

/* Here we call the headers_complete callback. This is somewhat
/* Here we call the headers_complete callback. This is somewhat
* different than other callbacks because if the user returns 1, we
* will interpret that as saying that this message has no body. This
* is needed for the annoying case of receiving a response to a HEAD
Expand Down Expand Up @@ -891,6 +925,13 @@ export class HTTP {

n('chunk_size_otherwise')
.match('\r', n('chunk_size_almost_done'))
.match(
'\n',
checkIfAllowLFWithoutCR(
n('chunk_size_almost_done'),
p.error(ERROR.CR_EXPECTED, 'Missing expected CR after chunk size'),
),
)
.match(';', n('chunk_extensions'))
.otherwise(p.error(ERROR.INVALID_CHUNK_SIZE,
'Invalid character in chunk size'));
Expand Down Expand Up @@ -923,6 +964,14 @@ export class HTTP {
.peek('\r', this.span.chunkExtensionName.end().skipTo(
onChunkExtensionNameCompleted(n('chunk_size_almost_done')),
))
.peek('\n', this.span.chunkExtensionName.end(
onChunkExtensionNameCompleted(
checkIfAllowLFWithoutCR(
n('chunk_size_almost_done'),
p.error(ERROR.CR_EXPECTED, 'Missing expected CR after chunk extension name'),
),
),
))
.otherwise(this.span.chunkExtensionName.end().skipTo(
p.error(ERROR.STRICT, 'Invalid character in chunk extensions name'),
));
Expand All @@ -936,6 +985,14 @@ export class HTTP {
.peek('\r', this.span.chunkExtensionValue.end().skipTo(
onChunkExtensionValueCompleted(n('chunk_size_almost_done')),
))
.peek('\n', this.span.chunkExtensionValue.end(
onChunkExtensionValueCompleted(
checkIfAllowLFWithoutCR(
n('chunk_size_almost_done'),
p.error(ERROR.CR_EXPECTED, 'Missing expected CR after chunk extension value'),
),
),
))
.otherwise(this.span.chunkExtensionValue.end().skipTo(
p.error(ERROR.STRICT, 'Invalid character in chunk extensions value'),
));
Expand All @@ -952,6 +1009,13 @@ export class HTTP {
n('chunk_extension_quoted_value_done')
.match(';', n('chunk_extensions'))
.match('\r', n('chunk_size_almost_done'))
.peek(
'\n',
checkIfAllowLFWithoutCR(
n('chunk_size_almost_done'),
p.error(ERROR.CR_EXPECTED, 'Missing expected CR after chunk extension value'),
),
)
.otherwise(p.error(ERROR.STRICT,
'Invalid character in chunk extensions quote value'));

Expand Down Expand Up @@ -979,6 +1043,13 @@ export class HTTP {

n('chunk_data_almost_done')
.match('\r\n', n('chunk_complete'))
.match(
'\n',
checkIfAllowLFWithoutCR(
n('chunk_complete'),
p.error(ERROR.CR_EXPECTED, 'Missing expected CR after chunk data'),
),
)
.otherwise(
this.testLenientFlags(LENIENT_FLAGS.OPTIONAL_CRLF_AFTER_CHUNK, {
1: n('chunk_complete'),
Expand Down
8 changes: 8 additions & 0 deletions src/native/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,14 @@ void llhttp_set_lenient_optional_crlf_after_chunk(llhttp_t* parser, int enabled)
}
}

void llhttp_set_lenient_optional_cr_before_lf(llhttp_t* parser, int enabled) {
if (enabled) {
parser->lenient_flags |= LENIENT_OPTIONAL_CR_BEFORE_LF;
} else {
parser->lenient_flags &= ~LENIENT_OPTIONAL_CR_BEFORE_LF;
}
}

/* Callbacks */


Expand Down
Loading