Skip to content

content_encoding: per-phase writer count breaks cumulative MAX_ENCODE_STACK cap #21603

@MegaManSec

Description

@MegaManSec

I did this

Hi all,

Small drift in the MAX_ENCODE_STACK = 5 chained-decoder cap in lib/content_encoding.c, plus a failf that's off by one:

The 2022 fix 3a09fbb7f2 ("content_encoding: return error on too many compression steps") used a per-call counter. Feb-2023 119fb18719 ("content_encoding: do not reset stage counter for each header") tightened that to a chain-cumulative counter via k->writer_stack_depth and added test418. The Oct-2023 refactor ad051e1cbe ("lib: client writer, part 2, accounting + logging") replaced writer_stack_depth with Curl_cwriter_count(data, phase) — which only counts writers in the matching phase. CURL_CW_TRANSFER_DECODE and CURL_CW_CONTENT_DECODE are distinct enums in lib/sendf.h:101-107, so Transfer-Encoding: gzip,gzip,gzip,gzip and Content-Encoding: gzip,gzip,gzip,gzip no longer see each other. test418 (same-phase) still passes.

The check at lib/content_encoding.c:753-757:

if(Curl_cwriter_count(data, phase) + 1 >= MAX_ENCODE_STACK) {
  failf(data, "Reject response due to more than %d content encodings",
        MAX_ENCODE_STACK);
  return CURLE_BAD_CONTENT_ENCODING;
}

Two things:

  1. + 1 >= 5 rejects at count==4, so the realised per-phase cap is 4, not 5. The failf "more than %d" text is misleading.
  2. Per-phase counting drifts from 119fb18719's cumulative intent.

Minimal fix to restore cumulative-across-phases:

-      if(Curl_cwriter_count(data, phase) + 1 >= MAX_ENCODE_STACK) {
-        failf(data, "Reject response due to more than %d content encodings",
+      size_t total = Curl_cwriter_count(data, CURL_CW_TRANSFER_DECODE) +
+        Curl_cwriter_count(data, CURL_CW_CONTENT_DECODE);
+      if(total + 1 >= MAX_ENCODE_STACK) {
+        failf(data, "Reject response due to more than %d encodings (total)",
               MAX_ENCODE_STACK);
         return CURLE_BAD_CONTENT_ENCODING;
       }

Separately, the decoders at lib/content_encoding.c:166-221 (zlib), :405-447 (brotli), :514-551 (zstd) don't poll Curl_timeleft inside their inner loops, so --max-time 30 doesn't fire mid-decode on a slow chain — verified with an 8-decoder/100 GiB body, --max-time 30 returned after ~3:18 once the chain drained. CURLOPT_MAXFILESIZE since 8.20.0 (77ed315096) already caps decompressed bytes and docs/KNOWN_RISKS.md documents it as the compression-bomb mitigation, so this is more a timeout-responsiveness gap than a bomb-defence question — but a Curl_timeleft poll inside the decoder loops would close it.

I expected the following

No response

curl/libcurl version

master

operating system

all

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions