Repository navigation
Inspect Flask bodies read from the raw stream - #783
iacobdaniel wants to merge 4 commits into
Conversation
| body = read_up_to(self._stream, MAX_BUFFERED_BODY_SIZE + 1) | ||
| if len(body) > MAX_BUFFERED_BODY_SIZE: | ||
| return io.BufferedReader(PrefixedStream(body, self._stream)) |
There was a problem hiding this comment.
🟡 Medium - Chunked raw reads allocate 20 MiB before returning data
For a terminated or chunked request without a declared length (or with a declared length at or below the threshold), the first application read consumes up to 20 MiB plus one byte before returning even one requested byte. An unauthenticated client can keep many streaming requests incomplete, forcing each worker to retain a large prefix and delaying streaming handlers, causing substantial memory and request-availability exhaustion.
Show fix
Do not read a threshold-sized prefix synchronously on the application's first read when the declared length is absent or unreliable. Either leave such streams unbuffered and explicitly mark them unscanned, or spool them with a bounded, configurable mechanism that applies backpressure without retaining one large in-memory prefix per request.
More info - Reply on this comment to give feedback or ignore the issue.
There was a problem hiding this comment.
Partially fixed in commit 8c0deb3.
This issue remains because BufferedWsgiInput._open_source still calls read_up_to(self._stream, MAX_BUFFERED_BODY_SIZE + 1) at line 97 for terminated streams, and read_up_to merely splits that synchronous read into 64 KiB chunks; it still retains the prefix and delays the first application byte until EOF or the threshold.
There was a problem hiding this comment.
This part is intended: Zen needs the whole body before the app acts on it, otherwise an attack early in a body the app parses while reading would get through. The wait is bounded by the 20 MB limit, memory only grows with the bytes the client sends, and get_json() already waits for and holds the full body the same way.
There was a problem hiding this comment.
Partially fixed in commit 524ce13.
This issue remains because BufferedWsgiInput._open_source still calls read_up_to(self._stream, MAX_BUFFERED_BODY_SIZE + 1) at aikido_zen/sources/flask/buffered_wsgi_input.py:109 for terminated/chunked requests, and reads the entire declared length at line 113 when it is at or below the threshold, before returning the first application read; the 64 KiB chunks only reduce per-read allocation, not synchronous prefetch or retained buffering.
There was a problem hiding this comment.
Intended, as explained above.
Bodies the app reads straight from request.stream or wsgi.input were not inspected. They are now read in full on the app's first read and stored for inspection, and later reads come from that copy.