From fc70ce08f5818a286fb5899a1bc3aff5965a745e Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 18 Nov 2020 20:50:21 -0800 Subject: [PATCH] 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. CVE-ID: CVE-2020-8287 PR-URL: https://github.com/nodejs-private/node-private/pull/235 Reviewed-By: Fedor Indutny --- http_parser.c | 7 +++++++ test.c | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/http_parser.c b/http_parser.c index 9be003e7322..e9b2b9e83b9 100644 --- a/http_parser.c +++ b/http_parser.c @@ -1344,6 +1344,13 @@ size_t http_parser_execute (http_parser *parser, } else if (parser->index == sizeof(TRANSFER_ENCODING)-2) { parser->header_state = h_transfer_encoding; parser->uses_transfer_encoding = 1; + + /* 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 + */ + parser->flags &= ~F_CHUNKED; } break; diff --git a/test.c b/test.c index 3f7c77b3494..2e5a9ebd678 100644 --- a/test.c +++ b/test.c @@ -2154,6 +2154,32 @@ const struct message responses[] = ,.body= "2\r\nOK\r\n0\r\n\r\n" ,.num_chunks_complete= 0 } +#define HTTP_200_DUPLICATE_TE_NOT_LAST_CHUNKED 30 +, {.name= "HTTP 200 response with `chunked` and duplicate Transfer-Encoding" + ,.type= HTTP_RESPONSE + ,.raw= "HTTP/1.1 200 OK\r\n" + "Transfer-Encoding: chunked\r\n" + "Transfer-Encoding: identity\r\n" + "\r\n" + "2\r\n" + "OK\r\n" + "0\r\n" + "\r\n" + ,.should_keep_alive= FALSE + ,.message_complete_on_eof= TRUE + ,.http_major= 1 + ,.http_minor= 1 + ,.status_code= 200 + ,.response_status= "OK" + ,.content_length= -1 + ,.num_headers= 2 + ,.headers= + { { "Transfer-Encoding", "chunked" } + , { "Transfer-Encoding", "identity" } + } + ,.body= "2\r\nOK\r\n0\r\n\r\n" + ,.num_chunks_complete= 0 + } }; /* strnlen() is a POSIX.2008 addition. Can't rely on it being available so &showmsg=1'>Expand)Author 2023-05-06services: dbus-service, secret-service: Do not cause (fibers) to be loaded....* gnu/build/dbus-service.scm (sleep*): Pass #:ensure #f to 'resolve-module'. * gnu/build/secret-service.scm (wait-for-readable-fd): Likewise. Ludovic Courtès 2022-06-10services: jami: Modernize to adjust to Shepherd 0.9+ changes....This partially fixes <https://issues.guix.gnu.org/54786>, allowing the 'jami' and 'jami-provisioning' system tests to pass again. In version 0.9.0, Shepherd constructors are now run concurrently, via cooperative scheduling (Guile Fibers). The Jami service previously relied on blocking sleeps while polling for D-Bus services to become ready after forking a process; this wouldn't work anymore since while blocking the service process wouldn't be given the chance to finish starting. The new reliance on Fibers in Shepherd's fork+exec-command in the helper 'send-dbus' procedure also meant that it wouldn't work outside of Shepherd anymore. Finally, the 'start-service' Shepherd procedure used in the test suite would cause the Jami daemon to be spawned multiple times (a bug introduced in Shepherd 0.9.0). To fix/simplify these problems, this change does the following: 1. Use the Guile AC/D-Bus library for D-Bus communication, which simplify things, such as avoiding the need to fork 'dbus-send' processes. 2. The non-blocking 'sleep' version of Fiber is used for the 'with-retries' waiting syntax. 3. A 'dbus' package variant is used to adjust the session bus configuration, tailoring it for the use case at hand. 4. Avoid start-service in the tests, preferring 'jami-service-available?' for now. * gnu/build/jami-service.scm (parse-dbus-reply, strip-quotes) (deserialize-item, serialize-boolean, dbus-dict->alist) (dbus-array->list, parse-account-ids, parse-account-details) (parse-contacts): Delete procedures. (%send-dbus-binary, %send-dbus-bus, %send-dbus-user, %send-dbus-group) (%send-dbus-debug): Delete parameters. (jami-service-running?): New procedure. (send-dbus/configuration-manager): Rename to... (call-configuration-manager-method): ... this. Turn METHOD into a positional argument. Turn ARGUMENTS into an optional argument. Invoke `call-dbus-method' instead of `send-dbus', adjusting callers accordingly. (get-account-ids, id->account-details, id->account-details) (id->volatile-account-details, username->id, add-account remove-account) (username->contacts, remove-contact, add-contact, set-account-details) (set-all-moderators, username->all-moderators?, username->moderators) (set-moderator): Adjust accordingly. (with-retries, send-dbus, dbus-available-services) (dbus-service-available?): Move to ... * gnu/build/dbus-service.scm: ... this new module. (send-dbus): Rewrite to use the Guile AC/D-Bus library. (%dbus-query-timeout, sleep*): New variables. (%current-dbus-connection): New parameter. (initialize-dbus-connection!, argument->signature-type) (call-dbus-method): New procedures. (dbus-available-services): Adjust accordingly. * gnu/local.mk (GNU_SYSTEM_MODULES): Register new module. * gnu/packages/glib.scm (dbus-for-jami): New variable. * gnu/services/telephony.scm: (jami-configuration)[dbus]: Default to dbus-for-jami. (jami-dbus-session-activation): Write a D-Bus daemon configuration file at '/var/run/jami/session-local.conf'. (jami-shepherd-services): Add the closure of guile-ac-d-bus and guile-fibers as extensions. Adjust imported modules. Remove no longer used parameters. <jami-dbus-session>: Use a PID file, avoiding the need for the manual synchronization. <jami>: Set DBUS_SESSION_BUS_ADDRESS environment variable. Poll using 'jami-service-available?' instead of 'dbus-service-available?'. * gnu/tests/telephony.scm (run-jami-test): Add needed Guile extensions. Set DBUS_SESSION_BUS_ADDRESS environment variable. Adjust all tests to use 'jami-service-available?' to determine if the service is started rather than the now problematic Shepherd's 'start-service'. Maxim Cournoyer