Commit 27ca73eb authored by Yann Ylavic's avatar Yann Ylavic
Browse files

Merge r892678, r1100511, r1102124 from trunk: 

Reject requests containing (invalid) NULL characters in request line
or request headers.
PR 43039


use APR_STATUS_IS_TIMEUP() instead of direct comparison with APR_TIMEUP.


Use APR_STATUS_IS_... in some more cases.

While this is not strictly necessary everywhere, it makes it much easier
to find the problematic cases.


Submitted by: niq, covener, sf
Reviewed  by: wrowe, covener, ylavic


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@1758671 13f79535-47bb-0310-9956-ffa450edef68
parent a743f731

CHANGES

+3 −0
Original line number Diff line number Diff line
@@ -4,6 +4,9 @@ Changes with Apache 2.2.32 
  *) core: CVE-2016-5387: Mitigate [f]cgi "httpoxy" issues.

     [Dominic Scheirlinck <dominic vendhq.com>, Yann Ylavic]



  *) Core: reject NULLs in request line or request headers.

     PR 43039 [Nick Kew]



  *) mod_ssl: Fix a possible memory leak on restart for custom [EC]DH params.

     [Jan Kaluza, Yann Ylavic]

STATUS

+0 −13
Original line number Diff line number Diff line
@@ -103,19 +103,6 @@ RELEASE SHOWSTOPPERS: 
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:

  [ start all new proposals below, under PATCHES PROPOSED. ]



  *) core: Reject requests containing (invalid) NULL characters in request line

     or request headers. (Including embedded %00 in URL).

     (Use APR_STATUS_IS_... in some more cases.)

     Trunk version of patch

         http://svn.apache.org/r892678

         http://svn.apache.org/r1100511

         http://svn.apache.org/r1102124

     Backport: (trunk works as well)

         https://raw.githubusercontent.com/wrowe/patches/master/backport-2.2.x-r892678.patch

     Submitted by niq, status legibility fixes by covener, sf

     PR: 43039

     +1: wrowe, covener, ylavic



  *) core: Limit to ten the number of tolerated empty lines between request.

     Before this commit, the maximum number of empty lines was the same as

     configured LimitRequestFields, defaulting to 100, which was way too much.

server/protocol.c

+20 −5
Original line number Diff line number Diff line
@@ -433,8 +433,13 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n, 
            }

        }

    }



    *read = bytes_handled;



    /* PR#43039: We shouldn't accept NULL bytes within the line */

    if (strlen(*s) < bytes_handled - 1) {

        return APR_EINVAL;

    }



    return APR_SUCCESS;

}
@@ -597,12 +602,15 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb) 
             * buffer before finding the end-of-line.  This is only going to

             * happen if it exceeds the configured limit for a request-line.

             */

            if (rv == APR_ENOSPC) {

            if (APR_STATUS_IS_ENOSPC(rv)) {

                r->status    = HTTP_REQUEST_URI_TOO_LARGE;

            }

            else if (APR_STATUS_IS_TIMEUP(rv)) {

                r->status = HTTP_REQUEST_TIME_OUT;

            }

            else if (APR_STATUS_IS_EINVAL(rv)) {

                r->status = HTTP_BAD_REQUEST;

            }

            r->proto_num = HTTP_VERSION(1,0);

            r->protocol  = apr_pstrdup(r->pool, "HTTP/1.0");

            return 0;
@@ -908,9 +916,16 @@ request_rec *ap_read_request(conn_rec *conn) 


    /* Get the request... */

    if (!read_request_line(r, tmp_bb)) {

        if (r->status == HTTP_REQUEST_URI_TOO_LARGE) {

        if (r->status == HTTP_REQUEST_URI_TOO_LARGE

            || r->status == HTTP_BAD_REQUEST) {

            if (r->status == HTTP_BAD_REQUEST) {

                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,

                              "request failed: invalid characters in URI");

            }

            else {

                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,

                              "request failed: URI too long (longer than %d)", r->server->limit_req_line);

            }

            ap_send_error_response(r, 0);

            ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);

            ap_run_log_transaction(r);