From 4cf751dbb992d73c70687e93ba38335121c4194b Mon Sep 17 00:00:00 2001 From: Rainer Jung Date: Thu, 16 Aug 2012 17:54:50 +0000 Subject: [PATCH] Fix closing the back end connection in case of error. The field "closed" was changed from an int to a bit field of size one in 2.4.x. For historical reasons a close instruction was coded as an increment on the field, which in 2.4.x flips the field each time. There were mutliple code paths that would flip it several times for a single error, so effectively the connection was no longer closed in these cases. Especially in the case of an aborted client connection this lead to a non consumed back end buffer and thus to response mixup between users. PR 53727 CVE-2012-3052 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1373955 13f79535-47bb-0310-9956-ffa450edef68 --- modules/proxy/mod_proxy_ajp.c | 26 +++++++++++++------------- modules/proxy/mod_proxy_http.c | 21 +++++++++++---------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/modules/proxy/mod_proxy_ajp.c b/modules/proxy/mod_proxy_ajp.c index c9f58171f5..3736156afc 100644 --- a/modules/proxy/mod_proxy_ajp.c +++ b/modules/proxy/mod_proxy_ajp.c @@ -207,7 +207,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, /* send request headers */ status = ajp_send_header(conn->sock, r, maxsize, uri); if (status != APR_SUCCESS) { - conn->close++; + conn->close = 1; ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00868) "request failed to %pI (%s)", conn->worker->cp->addr, @@ -234,7 +234,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, status = ajp_alloc_data_msg(r->pool, &buff, &bufsiz, &msg); if (status != APR_SUCCESS) { /* We had a failure: Close connection to backend */ - conn->close++; + conn->close = 1; ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00869) "ajp_alloc_data_msg failed"); return HTTP_INTERNAL_SERVER_ERROR; @@ -255,7 +255,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, if (status != APR_SUCCESS) { /* We had a failure: Close connection to backend */ - conn->close++; + conn->close = 1; ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00871) "ap_get_brigade failed"); apr_brigade_destroy(input_brigade); @@ -275,7 +275,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, status = apr_brigade_flatten(input_brigade, buff, &bufsiz); if (status != APR_SUCCESS) { /* We had a failure: Close connection to backend */ - conn->close++; + conn->close = 1; apr_brigade_destroy(input_brigade); ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00874) "apr_brigade_flatten"); @@ -290,7 +290,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, ajp_msg_log(r, msg, "First ajp_send_data_msg: ajp_ilink_send packet dump"); if (status != APR_SUCCESS) { /* We had a failure: Close connection to backend */ - conn->close++; + conn->close = 1; apr_brigade_destroy(input_brigade); ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00876) "send failed to %pI (%s)", @@ -319,7 +319,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, * for later resusage by the next request again. * Close it to clean things up. */ - conn->close++; + conn->close = 1; return HTTP_BAD_REQUEST; } } @@ -330,7 +330,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, (ajp_msg_t **)&(conn->data)); if (status != APR_SUCCESS) { /* We had a failure: Close connection to backend */ - conn->close++; + conn->close = 1; apr_brigade_destroy(input_brigade); ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00878) "read response failed from %pI (%s)", @@ -559,7 +559,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, * But: Close this connection to the backend. */ if (r->connection->aborted) { - conn->close++; + conn->close = 1; output_failed = 0; result = CMD_AJP13_END_RESPONSE; request_ended = 1; @@ -597,7 +597,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, "Processing of request failed backend: %i, " "output: %i", backend_failed, output_failed); /* We had a failure: Close connection to backend */ - conn->close++; + conn->close = 1; /* Return DONE to avoid error messages being added to the stream */ if (data_sent) { rv = DONE; @@ -607,7 +607,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00891) "Processing of request didn't terminate cleanly"); /* We had a failure: Close connection to backend */ - conn->close++; + conn->close = 1; backend_failed = 1; /* Return DONE to avoid error messages being added to the stream */ if (data_sent) { @@ -616,7 +616,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, } else if (!conn_reuse) { /* Our backend signalled connection close */ - conn->close++; + conn->close = 1; } else { ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00892) @@ -679,7 +679,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r, apr_brigade_destroy(output_brigade); if (apr_table_get(r->subprocess_env, "proxy-nokeepalive")) { - conn->close++; + conn->close = 1; } return rv; @@ -758,7 +758,7 @@ static int proxy_ajp_handler(request_rec *r, proxy_worker *worker, * TCP connection gets closed and try it once again. */ if (status != APR_SUCCESS) { - backend->close++; + backend->close = 1; ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897) "cping/cpong failed to %pI (%s)", worker->cp->addr, worker->s->hostname); diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index b219854400..243c9a5e7d 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -795,14 +795,14 @@ int ap_proxy_http_request(apr_pool_t *p, request_rec *r, } buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.0" CRLF, NULL); force10 = 1; - p_conn->close++; + p_conn->close = 1; } else { buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.1" CRLF, NULL); force10 = 0; } if (apr_table_get(r->subprocess_env, "proxy-nokeepalive")) { origin->keepalive = AP_CONN_CLOSE; - p_conn->close++; + p_conn->close = 1; } ap_xlate_proto_to_ascii(buf, strlen(buf)); e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc); @@ -1019,7 +1019,7 @@ int ap_proxy_http_request(apr_pool_t *p, request_rec *r, */ if (!r->kept_body && r->main) { /* XXX: Why DON'T sub-requests use keepalives? */ - p_conn->close++; + p_conn->close = 1; if (old_cl_val) { old_cl_val = NULL; apr_table_unset(r->headers_in, "Content-Length"); @@ -1056,7 +1056,7 @@ int ap_proxy_http_request(apr_pool_t *p, request_rec *r, apr_table_unset(r->headers_in, "Content-Length"); old_cl_val = NULL; origin->keepalive = AP_CONN_CLOSE; - p_conn->close++; + p_conn->close = 1; } /* Prefetch MAX_MEM_SPOOL bytes @@ -1705,7 +1705,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01106) "bad HTTP/%d.%d header returned by %s (%s)", major, minor, r->uri, r->method); - backend->close += 1; + backend->close = 1; /* * ap_send_error relies on a headers_out to be present. we * are in a bad position here.. so force everything we send out @@ -1746,7 +1746,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, "server %s:%d returned Transfer-Encoding" " and Content-Length", backend->hostname, backend->port); - backend->close += 1; + backend->close = 1; } /* @@ -1755,8 +1755,9 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, */ te = apr_table_get(r->headers_out, "Transfer-Encoding"); /* strip connection listed hop-by-hop headers from response */ - backend->close += ap_find_token(p, - apr_table_get(r->headers_out, "Connection"), "close"); + if (ap_find_token(p, apr_table_get(r->headers_out, "Connection"), + "close")) + backend->close = 1; ap_proxy_clear_connection(p, r->headers_out); if ((buf = apr_table_get(r->headers_out, "Content-Type"))) { ap_set_content_type(r, apr_pstrdup(p, buf)); @@ -1801,7 +1802,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, /* cancel keepalive if HTTP/1.0 or less */ if ((major < 1) || (minor < 1)) { - backend->close += 1; + backend->close = 1; origin->keepalive = AP_CONN_CLOSE; } } else { @@ -1809,7 +1810,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r, backasswards = 1; r->status = 200; r->status_line = "200 OK"; - backend->close += 1; + backend->close = 1; } if (ap_is_HTTP_INFO(proxy_status)) { -- 2.40.0