From 10342f684711600acafbadd8bd99b7f70185e21e Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Mon, 1 Jun 2020 15:40:03 +0000 Subject: [PATCH] mod_proxy_http: put request/response splitting checks under the same comment. If stream_reqbody() detects a Content-Length vs bytes streamed mismatch it means that a module's filter played bad games, such inconsistency on the client connection would have been caught by ap_http_filter(). So move AH01087 logic to AH01086, under the same comment and log message, and always return HTTP_INTERNAL_SERVER because the client is not the culprit here. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1878367 13f79535-47bb-0310-9956-ffa450edef68 --- modules/proxy/mod_proxy_http.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index a4be4ee336a..8b93632341f 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -364,20 +364,26 @@ static int stream_reqbody(proxy_http_req_t *req) } } else if (rb_method == RB_STREAM_CL - && bytes_streamed > req->cl_val) { - /* C-L < bytes streamed?!? - * We will error out after the body is completely - * consumed, but we can't stream more bytes at the - * back end since they would in part be interpreted - * as another request! If nothing is sent, then - * just send nothing. + && (bytes_streamed > req->cl_val + || (seen_eos && bytes_streamed < req->cl_val))) { + /* C-L != bytes streamed?!? * - * Prevents HTTP Response Splitting. + * Prevent HTTP Request/Response Splitting. + * + * We can't stream more (or less) bytes at the back end since + * they could be interpreted in separate requests (more bytes + * now would start a new request, less bytes would make the + * first bytes of the next request be part of the current one). + * + * It can't happen from the client connection here thanks to + * ap_http_filter(), but some module's filter may be playing + * bad games, hence the HTTP_INTERNAL_SERVER_ERROR. */ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01086) - "read more bytes of request body than expected " + "read %s bytes of request body than expected " "(got %" APR_OFF_T_FMT ", expected " "%" APR_OFF_T_FMT ")", + bytes_streamed > req->cl_val ? "more" : "less", bytes_streamed, req->cl_val); return HTTP_INTERNAL_SERVER_ERROR; } @@ -403,13 +409,6 @@ static int stream_reqbody(proxy_http_req_t *req) } } while (!seen_eos); - if (rb_method == RB_STREAM_CL && bytes_streamed != req->cl_val) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01087) - "client %s given Content-Length did not match" - " number of body bytes read", r->connection->client_ip); - return HTTP_BAD_REQUEST; - } - return OK; } -- 2.47.3