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!
thanks,
greg k-h
