Re: svn commit: r1878092 - /httpd/httpd/trunk/server/scoreboard.c

[Yann Ylavic]

On Tue, Sep 7, 2021 at 2:00 PM Yann Ylavic <ylavic....@gmail.com> wrote:
>
> On Mon, May 25, 2020 at 7:50 AM <rpl...@apache.org> wrote:
> >
> > Author: rpluem
> > Date: Mon May 25 05:50:12 2020
> > New Revision: 1878092
> >
> > URL: http://svn.apache.org/viewvc?rev=1878092&view=rev
> > Log:
> > Fix a NULL pointer dereference
> >
> > * server/scoreboard.c (ap_increment_counts): In certain cases like certain
> >   invalid requests r->method might be NULL here. r->method_number defaults
> >   to M_GET and hence is M_GET in these cases.
> >
> >
> > Modified:
> >     httpd/httpd/trunk/server/scoreboard.c
> >
> > Modified: httpd/httpd/trunk/server/scoreboard.c
> > URL: 
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/scoreboard.c?rev=1878092&r1=1878091&r2=1878092&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/server/scoreboard.c (original)
> > +++ httpd/httpd/trunk/server/scoreboard.c Mon May 25 05:50:12 2020
> > @@ -381,7 +381,7 @@ AP_DECLARE(void) ap_increment_counts(ap_
> >      if (pfn_ap_logio_get_last_bytes != NULL) {
> >          bytes = pfn_ap_logio_get_last_bytes(r->connection);
> >      }
> > -    else if (r->method_number == M_GET && r->method[0] == 'H') {
> > +    else if (r->method_number == M_GET && r->method && r->method[0] == 
> > 'H') {
> >          bytes = 0;
> >      }
> >      else {
>
> Sorry for the lateness..
> Maybe we could have an r->method_number == M_INVALID and r->method ==
> "" by default on failure, like with the attached patch?

Or even initialize r->{method,uri,unparsed_uri} to "-" for logging (v2
attached).
We can ap_die() from here, but the potential ap_internal_redirect()
from there would rewrite those.

Index: server/protocol.c
===================================================================
--- server/protocol.c	(revision 1893001)
+++ server/protocol.c	(working copy)
@@ -716,6 +716,13 @@ static int read_request_line(request_rec *r, apr_b
         if (rv != APR_SUCCESS) {
             r->request_time = apr_time_now();
 
+            /* Fall through with an invalid (non NULL) request */
+            r->method = "-";
+            r->method_number = M_INVALID;
+            r->uri = r->unparsed_uri = apr_pstrdup(r->pool, "-");
+            r->proto_num = HTTP_VERSION(1,0);
+            r->protocol  = "HTTP/1.0";
+
             /* 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.
@@ -732,8 +739,6 @@ static int read_request_line(request_rec *r, apr_b
             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;
         }
     } while ((len <= 0) && (--num_blank_lines >= 0));