Skip to content

Commit

Permalink
New lenient flag to make CR completely optional. (#243)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ShogunPanda
ShogunPanda authored Sep 12, 2023
1 parent 926c982 commit 50524c0
Show file tree
Hide file tree
Showing 17 changed files with 404 additions and 97 deletions.
8 changes: 8 additions & 0 deletions README.md
View file
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
View file
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
View file
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
View file
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

0 comments on commit 50524c0

Please sign in to comment.