sahvx655-wq opened a new pull request, #3644: URL: https://github.com/apache/brpc/pull/3644
### What problem does this PR solve? Issue Number: none Problem Summary: `http_parser_execute()` sets `F_CHUNKED` when a `Transfer-Encoding` value ends in `chunked`, but nothing clears it when another `Transfer-Encoding` line follows. RFC 7230 3.2.2 treats repeated lines as one comma-separated list, so ``` Transfer-Encoding: chunked Transfer-Encoding: identity ``` means the same as `Transfer-Encoding: chunked, identity`. A request with the one-line form is already rejected with `HPE_INVALID_TRANSFER_ENCODING` (RFC 7230 3.3.3, chunked is not the final coding). The two-line form is accepted and its body is read as chunked. A front-end that merges the lines, or uses the last one, sees a different message boundary than bRPC. That is the request smuggling setup behind CVE-2020-8287 in Node's http-parser. On the client side, a response with these headers is chunk-decoded where it should be read until EOF. This is a sibling of the framing cases fixed in #3521 (space before colon, `1 3`, obs-fold), which did not cover a repeated header. Repro on master: `POST / HTTP/1.1\r\nHost: a.com\r\nTransfer-Encoding: chunked\r\nTransfer-Encoding: identity\r\n\r\n5\r\nhello\r\n0\r\n\r\n` parses as one complete request with body `hello` and errno 0. Joining the two values into `chunked, identity` fails with errno 31. ### What is changed and the side effects? Changed: - Clear `F_CHUNKED` when a `Transfer-Encoding` value starts, the same reset llhttp does, so only the final coding decides framing. `gzip` and then `chunked` on separate lines still parses as chunked. - Add `HttpMessageTest.repeated_transfer_encoding_lines`. It fails on master and passes with this change. `brpc_http_message_unittest`, `brpc_http_parser_unittest`, `brpc_http_rpc_protocol_unittest`, `brpc_grpc_protocol_unittest` and `brpc_uri_unittest` pass locally (macOS, CMake). Side effects: - Performance effects: none, one flag clear per `Transfer-Encoding` line. - Breaking backward compatibility: a request whose last `Transfer-Encoding` line does not end in `chunked`, after an earlier line that did, now gets 400, as the one-line form already does. A response of that shape is read until EOF. --- ### Check List: - Please make sure your changes are compilable. - When providing us with a new feature, it is best to add related tests. - Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
