Le 10/04/2026 à 9:35 AM, Greg Kroah-Hartman a écrit :
On Fri, Apr 10, 2026 at 09:29:59AM +0200, Christopher Faulet wrote:
Le 09/04/2026 à 1:06 PM, Greg Kroah-Hartman a écrit :
FCGI content_length is a 16-bit field but fcgi_set_record_size()
is called with size_t/uint32_t arguments. With tune.bufsize >= 65544
(legal; cfgparse-global.c only enforces <= INT_MAX-16), a single
HTX DATA block or accumulated outbuf can exceed 65535 bytes. The
implicit conversion to uint16_t silently truncates the length field
while b_add(mbuf, outbuf.data) writes the full body.
A client posting ~99000 bytes can craft the body so that bytes
after the truncated length are parsed by PHP-FPM as fresh FCGI
records on the connection: a smuggled BEGIN_REQUEST + PARAMS with
arbitrary SCRIPT_FILENAME / PHP_VALUE bypasses all haproxy ACLs.
Fix the zero-copy path by refusing it when the block exceeds 65535
bytes (falls through to copy). Fix the copy path by capping
outbuf.size to 65535 + header so the data-fill loop naturally
stops at the FCGI maximum and emits the rest in a subsequent record.
The PARAMS path at line 2084 is similarly affected but harder to
trigger (requires combined header+param size > 65535) and is
covered by the same outbuf.size cap pattern if applied there;
left for a follow-up since the STDIN path is the practical attack.
---
src/mux_fcgi.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index acb06a7637b3..f57e1a5fa99c 100644
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -2153,7 +2153,14 @@ static size_t fcgi_strm_send_stdin(struct fcgi_conn
*fconn, struct fcgi_strm *fs
goto end;
type = htx_get_blk_type(blk);
size = htx_get_blksz(blk);
- if (unlikely(size == count && b_size(mbuf) == b_size(buf) &&
+ /* FCGI content_len is uint16_t. With tune.bufsize >= 65544 a single
+ * HTX block can exceed 65535 bytes; the implicit truncation in
+ * fcgi_set_record_size() would then desynchronize the record
+ * stream and let the client smuggle a forged FCGI request to the
+ * backend. Refuse zero-copy in that case and let the copy path
+ * split the data across multiple records.
+ */
+ if (unlikely(size <= 0xFFFF && size == count && b_size(mbuf) == b_size(buf)
&&
htx_nbblks(htx) == 1 && type == HTX_BLK_DATA)) {
void *old_area = mbuf->area;
int eom = (htx->flags & HTX_FL_EOM);
@@ -2212,6 +2219,11 @@ static size_t fcgi_strm_send_stdin(struct fcgi_conn
*fconn, struct fcgi_strm *fs
if (outbuf.size < FCGI_RECORD_HEADER_SZ + extra_bytes)
goto full;
+ /* FCGI content_len is uint16_t; cap output to avoid truncation in
+ * fcgi_set_record_size() when tune.bufsize is large. */
+ if (outbuf.size > 0xFFFF + FCGI_RECORD_HEADER_SZ)
+ outbuf.size = 0xFFFF + FCGI_RECORD_HEADER_SZ;
+
/* vsn: 1(FCGI_VERSION), type: (5)FCGI_STDIN, id: fstrm->id,
* len: 0x0000 (fill later), padding: 0x00, rsv: 0x00 */
memcpy(outbuf.area, "\x01\x05\x00\x00\x00\x00\x00\x00",
FCGI_RECORD_HEADER_SZ);
This one is valid too, but after a quick review, I guess
fcgi_strm_send_params() must also be fixed. If you agree, I can amend the
patch. But if you prefer to take a look and resubmit a new patch, I'm ok
too. Let me know what you prefer.
If you want to amend the patch, that would be great, no objection from
me at all!
Now merged, thanks !
--
Christopher Faulet
