101 lines
3.8 KiB
Diff
101 lines
3.8 KiB
Diff
This patch comes from upstream. It corresponds to a patch applied to
|
|
the generated C source code for llhttp included in Node.js 14.16.0
|
|
(see commit 641f786bb1a1f6eb1ff8750782ed939780f2b31a). That commit
|
|
fixes CVE-2020-8287. With this patch, the output of our
|
|
llhttp-bootstrap package matches the files included in Node.js 14.16.0
|
|
exactly.
|
|
|
|
commit e9b36ea64709c35ca66094d5cf3787f444029601
|
|
Author: Fedor Indutny <fedor@indutny.com>
|
|
Date: Sat Oct 10 19:56:01 2020 -0700
|
|
|
|
http: unset `F_CHUNKED` on new `Transfer-Encoding`
|
|
|
|
Duplicate `Transfer-Encoding` header should be a treated as a single,
|
|
but with original header values concatenated with a comma separator. In
|
|
the light of this, even if the past `Transfer-Encoding` ended with
|
|
`chunked`, we should be not let the `F_CHUNKED` to leak into the next
|
|
header, because mere presence of another header indicates that `chunked`
|
|
is not the last transfer-encoding token.
|
|
|
|
diff --git a/src/llhttp/http.ts b/src/llhttp/http.ts
|
|
index f4f1a6e..0a0c365 100644
|
|
--- a/src/llhttp/http.ts
|
|
+++ b/src/llhttp/http.ts
|
|
@@ -460,11 +460,19 @@ export class HTTP {
|
|
.match([ ' ', '\t' ], n('header_value_discard_ws'))
|
|
.otherwise(checkContentLengthEmptiness);
|
|
|
|
+ // Multiple `Transfer-Encoding` headers should be treated as one, but with
|
|
+ // values separate by a comma.
|
|
+ //
|
|
+ // See: https://tools.ietf.org/html/rfc7230#section-3.2.2
|
|
+ const toTransferEncoding = this.unsetFlag(
|
|
+ FLAGS.CHUNKED,
|
|
+ 'header_value_te_chunked');
|
|
+
|
|
n('header_value_start')
|
|
.otherwise(this.load('header_state', {
|
|
[HEADER_STATE.UPGRADE]: this.setFlag(FLAGS.UPGRADE, fallback),
|
|
[HEADER_STATE.TRANSFER_ENCODING]: this.setFlag(
|
|
- FLAGS.TRANSFER_ENCODING, 'header_value_te_chunked'),
|
|
+ FLAGS.TRANSFER_ENCODING, toTransferEncoding),
|
|
[HEADER_STATE.CONTENT_LENGTH]: n('header_value_content_length_once'),
|
|
[HEADER_STATE.CONNECTION]: n('header_value_connection'),
|
|
}, 'header_value'));
|
|
@@ -847,6 +855,11 @@ export class HTTP {
|
|
return span.start(span.end(this.node(next)));
|
|
}
|
|
|
|
+ private unsetFlag(flag: FLAGS, next: string | Node): Node {
|
|
+ const p = this.llparse;
|
|
+ return p.invoke(p.code.and('flags', ~flag), this.node(next));
|
|
+ }
|
|
+
|
|
private setFlag(flag: FLAGS, next: string | Node): Node {
|
|
const p = this.llparse;
|
|
return p.invoke(p.code.or('flags', flag), this.node(next));
|
|
diff --git a/test/request/transfer-encoding.md b/test/request/transfer-encoding.md
|
|
index a7d1681..b0891d6 100644
|
|
--- a/test/request/transfer-encoding.md
|
|
+++ b/test/request/transfer-encoding.md
|
|
@@ -353,6 +353,38 @@ off=106 headers complete method=3 v=1/1 flags=200 content_length=0
|
|
off=106 error code=15 reason="Request has invalid `Transfer-Encoding`"
|
|
```
|
|
|
|
+## POST with `chunked` and duplicate transfer-encoding
|
|
+
|
|
+<!-- meta={"type": "request", "noScan": true} -->
|
|
+```http
|
|
+POST /post_identity_body_world?q=search#hey HTTP/1.1
|
|
+Accept: */*
|
|
+Transfer-Encoding: chunked
|
|
+Transfer-Encoding: deflate
|
|
+
|
|
+World
|
|
+```
|
|
+
|
|
+```log
|
|
+off=0 message begin
|
|
+off=5 len=38 span[url]="/post_identity_body_world?q=search#hey"
|
|
+off=44 url complete
|
|
+off=54 len=6 span[header_field]="Accept"
|
|
+off=61 header_field complete
|
|
+off=62 len=3 span[header_value]="*/*"
|
|
+off=67 header_value complete
|
|
+off=67 len=17 span[header_field]="Transfer-Encoding"
|
|
+off=85 header_field complete
|
|
+off=86 len=7 span[header_value]="chunked"
|
|
+off=95 header_value complete
|
|
+off=95 len=17 span[header_field]="Transfer-Encoding"
|
|
+off=113 header_field complete
|
|
+off=114 len=7 span[header_value]="deflate"
|
|
+off=123 header_value complete
|
|
+off=125 headers complete method=3 v=1/1 flags=200 content_length=0
|
|
+off=125 error code=15 reason="Request has invalid `Transfer-Encoding`"
|
|
+```
|
|
+
|
|
## POST with `chunked` before other transfer-coding (lenient)
|
|
|
|
TODO(indutny): should we allow it even in lenient mode? (Consider disabling
|