Bug 63669

Summary: Incomplete error code check in read_request_line()
Product: Apache httpd-2 Reporter: legendt
Component: CoreAssignee: Apache HTTPD Bugs Mailing List <bugs>
Status: RESOLVED FIXED    
Severity: major Keywords: FixedInTrunk, PatchAvailable
Priority: P2    
Version: 2.5-HEAD   
Target Milestone: ---   
Hardware: PC   
OS: Mac OS X 10.1   
Attachments: APR_BADARG error handling

Description legendt 2019-08-16 11:13:21 UTC
at httpd/server/protocol.c around line

static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
{
...
        rv = ap_rgetline(&(r->the_request), (apr_size_t)(r->server->limit_req_line + 2),
                         &len, r, strict ? AP_GETLINE_CRLF : 0, bb);

        if (rv != APR_SUCCESS) {
            r->request_time = apr_time_now();

            /* ap_rgetline returns APR_ENOSPC if it fills up the
             * buffer before finding the end-of-line.  This is only going to
             * happen if it exceeds the configured limit for a request-line.
             */
            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  = "HTTP/1.0";
            return 0;
        }
...

However, the function ap_rgetline() can actually return error codes other than APR_ENOSPC, APR_TIMEUP, APR_EINVAL. If the input bb is NULL, it can even return APR_BADARG, and in some cases it returns APR_EGENERAL. These errors are ignored and HTTP status is not correctly set.
Comment 1 Giovanni Bechis 2019-12-28 16:11:34 UTC
Created attachment 36937 [details]
APR_BADARG error handling

APR_BADARG error handling, APR_EGENERAL case could not be easily handled because it is also triggered at the end of each keepalive connection and it happens when non-blocking is asked too.
(server/protocol.c, line 252)
Comment 2 Eric Covener 2020-01-31 02:16:27 UTC
Thanks Giovanni, applied to trunk in r1873394