Christoph Biedl wrote... > About next steps, I would do the upload in the next days. Let me know if > you prefer other things to happen first or instead.
To avoid this gets lost I've just uploaded http-parser 2.8.1-1+deb10u2.
Updated debiff attached, only editorial changes since the previous mail.
Regards,
Christoph
diff -Nru http-parser-2.8.1/debian/changelog http-parser-2.8.1/debian/changelog --- http-parser-2.8.1/debian/changelog 2021-06-04 20:59:56.000000000 +0200 +++ http-parser-2.8.1/debian/changelog 2021-10-31 23:50:09.000000000 +0100 @@ -1,3 +1,11 @@ +http-parser (2.8.1-1+deb10u2) buster; urgency=medium + + * Fix ABI breakage introduced by accident in 2.8.1-1+deb10u1. + Many thanks to Hilko Bengen. + Closes: #996460, #996939, #996997 + + -- Christoph Biedl <debian.a...@manchmal.in-ulm.de> Sun, 31 Oct 2021 23:50:09 +0100 + http-parser (2.8.1-1+deb10u1) buster; urgency=medium * Cherry-pick "Support multi-coding Transfer-Encoding". diff -Nru http-parser-2.8.1/debian/patches/fix-ABI-breakage.patch http-parser-2.8.1/debian/patches/fix-ABI-breakage.patch --- http-parser-2.8.1/debian/patches/fix-ABI-breakage.patch 1970-01-01 01:00:00.000000000 +0100 +++ http-parser-2.8.1/debian/patches/fix-ABI-breakage.patch 2021-10-31 23:50:09.000000000 +0100 @@ -0,0 +1,224 @@ +Subject: Fix ABI breakage introduced by accident in 2.8.1-1+deb10u1 +Author: Hilko Bengen <ben...@debian.org> +Date: 2021-10-22 +Bug-Debian: + https://bugs.debian.org/996460 + https://bugs.debian.org/996939 + https://bugs.debian.org/996997 +Comment: (by the http-parser maintainer) + + # Problem + + The fix for CVE-2019-15605 introduced an ABI break by changing the + layout of struct http_parser - a change that was needed to store a + ninth bit in the "flags" field. However, this affected offsets of + fields declared as public, causing applications to break. + + # Workaround + + To restore the previous layout while still implementing the fix: Steal + one bit from the (private) content_length field (the remaining 63 + are more than enough) to store the newly introduced flag. + + Also rename the related constant as a safeguard against applications + that use it (they must not, see below). + + # Possible regressions + + A lot of work was done to avoid damage for well-behaving applications. + It seems all applications in Debian built against http-parser fall + into that category. + + Applications however that access fields in struct http_parser that are + in the section marked "/** PRIVATE **/" may face issues. Such a + behaviour is inacceptable anyway. + + If such a mis-behaving application ... + + * was built using an earlier version of http-parser, the code will + assume content_length is a 64 bit value. Depending on endianess and + status of the F_TRANSFER_ENCODING bit, things may work. Possibly + they will not. + + * uses the private F_TRANSFER_ENCODING constant and was built using + http-parser 2.8.1-1+deb10u1, it will not see the information it + expects to see. + Additionally, and re-build will fail. This is by design. + + Again, applications must not access fields declared private, and their + authors should not expect pity if they encounter breakage any anything + changes there. + + # Acknowledgments + + Thanks a lot to Hilko Bengen for the idea, implementation, a first + round of tests and many suggestions. + +--- a/http_parser.c ++++ b/http_parser.c +@@ -25,9 +25,7 @@ + #include <string.h> + #include <limits.h> + +-#ifndef ULLONG_MAX +-# define ULLONG_MAX ((uint64_t) -1) /* 2^64-1 */ +-#endif ++#define CONTENT_LENGTH_MAX (((uint64_t)-1) >> 1) + + #ifndef MIN + # define MIN(a,b) ((a) < (b) ? (a) : (b)) +@@ -724,7 +722,8 @@ + if (ch == CR || ch == LF) + break; + parser->flags = 0; +- parser->content_length = ULLONG_MAX; ++ parser->flags2 = 0; ++ parser->content_length = CONTENT_LENGTH_MAX; + + if (ch == 'H') { + UPDATE_STATE(s_res_or_resp_H); +@@ -759,7 +758,8 @@ + case s_start_res: + { + parser->flags = 0; +- parser->content_length = ULLONG_MAX; ++ parser->flags2 = 0; ++ parser->content_length = CONTENT_LENGTH_MAX; + + switch (ch) { + case 'H': +@@ -923,7 +923,8 @@ + if (ch == CR || ch == LF) + break; + parser->flags = 0; +- parser->content_length = ULLONG_MAX; ++ parser->flags2 = 0; ++ parser->content_length = CONTENT_LENGTH_MAX; + + if (UNLIKELY(!IS_ALPHA(ch))) { + SET_ERRNO(HPE_INVALID_METHOD); +@@ -1314,7 +1315,7 @@ + parser->header_state = h_general; + } else if (parser->index == sizeof(TRANSFER_ENCODING)-2) { + parser->header_state = h_transfer_encoding; +- parser->flags |= F_TRANSFER_ENCODING; ++ parser->flags2 |= F_TRANSFER_ENCODING2; + } + break; + +@@ -1528,7 +1529,7 @@ + t += ch - '0'; + + /* Overflow? Test against a conservative limit for simplicity. */ +- if (UNLIKELY((ULLONG_MAX - 10) / 10 < parser->content_length)) { ++ if (UNLIKELY((CONTENT_LENGTH_MAX - 10) / 10 < parser->content_length)) { + SET_ERRNO(HPE_INVALID_CONTENT_LENGTH); + parser->header_state = h_state; + goto error; +@@ -1765,7 +1766,7 @@ + + /* Cannot us transfer-encoding and a content-length header together + per the HTTP specification. (RFC 7230 Section 3.3.3) */ +- if ((parser->flags & F_TRANSFER_ENCODING) && ++ if ((parser->flags2 & F_TRANSFER_ENCODING2) && + (parser->flags & F_CONTENTLENGTH)) { + /* Allow it for lenient parsing as long as `Transfer-Encoding` is + * not `chunked` +@@ -1834,7 +1835,7 @@ + parser->nread = 0; + + hasBody = parser->flags & F_CHUNKED || +- (parser->content_length > 0 && parser->content_length != ULLONG_MAX); ++ (parser->content_length > 0 && parser->content_length != CONTENT_LENGTH_MAX); + if (parser->upgrade && (parser->method == HTTP_CONNECT || + (parser->flags & F_SKIPBODY) || !hasBody)) { + /* Exit, the rest of the message is in a different protocol. */ +@@ -1850,7 +1851,7 @@ + /* chunked encoding - ignore Content-Length header, + * prepare for a chunk */ + UPDATE_STATE(s_chunk_size_start); +- } else if (parser->flags & F_TRANSFER_ENCODING) { ++ } else if (parser->flags2 & F_TRANSFER_ENCODING2) { + if (parser->type == HTTP_REQUEST && !lenient) { + /* RFC 7230 3.3.3 */ + +@@ -1877,7 +1878,7 @@ + /* Content-Length header given but zero: Content-Length: 0\r\n */ + UPDATE_STATE(NEW_MESSAGE()); + CALLBACK_NOTIFY(message_complete); +- } else if (parser->content_length != ULLONG_MAX) { ++ } else if (parser->content_length != CONTENT_LENGTH_MAX) { + /* Content-Length header given and non-zero */ + UPDATE_STATE(s_body_identity); + } else { +@@ -1901,7 +1902,7 @@ + (uint64_t) ((data + len) - p)); + + assert(parser->content_length != 0 +- && parser->content_length != ULLONG_MAX); ++ && parser->content_length != CONTENT_LENGTH_MAX); + + /* The difference between advancing content_length and p is because + * the latter will automaticaly advance on the next loop iteration. +@@ -1991,7 +1992,7 @@ + t += unhex_val; + + /* Overflow? Test against a conservative limit for simplicity. */ +- if (UNLIKELY((ULLONG_MAX - 16) / 16 < parser->content_length)) { ++ if (UNLIKELY((CONTENT_LENGTH_MAX - 16) / 16 < parser->content_length)) { + SET_ERRNO(HPE_INVALID_CONTENT_LENGTH); + goto error; + } +@@ -2035,7 +2036,7 @@ + + assert(parser->flags & F_CHUNKED); + assert(parser->content_length != 0 +- && parser->content_length != ULLONG_MAX); ++ && parser->content_length != CONTENT_LENGTH_MAX); + + /* See the explanation in s_body_identity for why the content + * length and data pointers are managed this way. +@@ -2124,12 +2125,12 @@ + } + + /* RFC 7230 3.3.3, see `s_headers_almost_done` */ +- if ((parser->flags & F_TRANSFER_ENCODING) && ++ if ((parser->flags2 & F_TRANSFER_ENCODING2) && + (parser->flags & F_CHUNKED) == 0) { + return 1; + } + +- if ((parser->flags & F_CHUNKED) || parser->content_length != ULLONG_MAX) { ++ if ((parser->flags & F_CHUNKED) || parser->content_length != CONTENT_LENGTH_MAX) { + return 0; + } + +--- a/http_parser.h ++++ b/http_parser.h +@@ -225,7 +225,7 @@ + , F_UPGRADE = 1 << 5 + , F_SKIPBODY = 1 << 6 + , F_CONTENTLENGTH = 1 << 7 +- , F_TRANSFER_ENCODING = 1 << 8 ++ , F_TRANSFER_ENCODING2 = 1 << 0 /* this goes into http_parser.flags2 */ + }; + + +@@ -296,14 +296,15 @@ + struct http_parser { + /** PRIVATE **/ + unsigned int type : 2; /* enum http_parser_type */ ++ unsigned int flags : 8; /* F_* values from 'flags' enum; semi-public */ + unsigned int state : 7; /* enum state from http_parser.c */ + unsigned int header_state : 7; /* enum header_state from http_parser.c */ + unsigned int index : 7; /* index into current matcher */ + unsigned int lenient_http_headers : 1; +- unsigned int flags : 16; /* F_* values from 'flags' enum; semi-public */ + + uint32_t nread; /* # bytes read in various scenarios */ +- uint64_t content_length; /* # bytes in body (0 if no Content-Length header) */ ++ uint64_t flags2 : 1; /* one bit another flag */ ++ uint64_t content_length : 63; /* # bytes in body (0 if no Content-Length header) */ + + /** READ-ONLY **/ + unsigned short http_major; diff -Nru http-parser-2.8.1/debian/patches/series http-parser-2.8.1/debian/patches/series --- http-parser-2.8.1/debian/patches/series 2021-05-24 10:46:26.000000000 +0200 +++ http-parser-2.8.1/debian/patches/series 2021-10-31 23:50:09.000000000 +0100 @@ -1,3 +1,4 @@ improve-installation.patch 1580760635.v2.9.2-2-g7d5c99d.support-multi-coding-transfer-encoding.patch +fix-ABI-breakage.patch
signature.asc
Description: PGP signature
