Commit ba553e70 authored by Graham Leggett's avatar Graham Leggett
Browse files

core: For consistency, ensure that read lines are NUL terminated on any

error, not only on buffer full.
trunk patch: http://svn.apache.org/r1824303
+1: ylavic, rpluem, minfrin


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1824469 13f79535-47bb-0310-9956-ffa450edef68
parent a5cd8847
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
                                                         -*- coding: utf-8 -*-
Changes with Apache 2.4.30

  *) core: For consistency, ensure that read lines are NUL terminated on any
     error, not only on buffer full.  [Yann Ylavic]

  *) mod_authnz_ldap: Fix language long names detection as short name.
     [Yann Ylavic]

+0 −6
Original line number Diff line number Diff line
@@ -118,12 +118,6 @@ RELEASE SHOWSTOPPERS:
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
  [ start all new proposals below, under PATCHES PROPOSED. ]

  *) core: For consistency, ensure that read lines are NUL terminated on any
     error, not only on buffer full.
     trunk patch: http://svn.apache.org/r1824303
     2.4.x patch: trunk works (modulo CHANGES)
     +1: ylavic, rpluem, minfrin


PATCHES PROPOSED TO BACKPORT FROM TRUNK:
  [ New proposals should be added at the end of the list ]
+42 −34
Original line number Diff line number Diff line
@@ -225,6 +225,11 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
    int fold = flags & AP_GETLINE_FOLD;
    int crlf = flags & AP_GETLINE_CRLF;

    if (!n) {
        /* Needs room for NUL byte at least */
        return APR_BADARG;
    }

    /*
     * Initialize last_char as otherwise a random value will be compared
     * against APR_ASCII_LF at the end of the loop if bb only contains
@@ -238,14 +243,15 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
        rv = ap_get_brigade(r->proto_input_filters, bb, AP_MODE_GETLINE,
                            APR_BLOCK_READ, 0);
        if (rv != APR_SUCCESS) {
            return rv;
            goto cleanup;
        }

        /* Something horribly wrong happened.  Someone didn't block! 
         * (this also happens at the end of each keepalive connection)
         */
        if (APR_BRIGADE_EMPTY(bb)) {
            return APR_EGENERAL;
            rv = APR_EGENERAL;
            goto cleanup;
        }

        for (e = APR_BRIGADE_FIRST(bb);
@@ -263,7 +269,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,

            rv = apr_bucket_read(e, &str, &len, APR_BLOCK_READ);
            if (rv != APR_SUCCESS) {
                return rv;
                goto cleanup;
            }

            if (len == 0) {
@@ -276,17 +282,8 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,

            /* Would this overrun our buffer?  If so, we'll die. */
            if (n < bytes_handled + len) {
                *read = bytes_handled;
                if (*s) {
                    /* ensure this string is NUL terminated */
                    if (bytes_handled > 0) {
                        (*s)[bytes_handled-1] = '\0';
                    }
                    else {
                        (*s)[0] = '\0';
                    }
                }
                return APR_ENOSPC;
                rv = APR_ENOSPC;
                goto cleanup;
            }

            /* Do we have to handle the allocation ourselves? */
@@ -294,7 +291,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
                /* We'll assume the common case where one bucket is enough. */
                if (!*s) {
                    current_alloc = len;
                    *s = apr_palloc(r->pool, current_alloc);
                    *s = apr_palloc(r->pool, current_alloc + 1);
                }
                else if (bytes_handled + len > current_alloc) {
                    /* Increase the buffer size */
@@ -305,7 +302,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
                        new_size = (bytes_handled + len) * 2;
                    }

                    new_buffer = apr_palloc(r->pool, new_size);
                    new_buffer = apr_palloc(r->pool, new_size + 1);

                    /* Copy what we already had. */
                    memcpy(new_buffer, *s, bytes_handled);
@@ -329,19 +326,15 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
        }
    }

    if (crlf && (last_char <= *s || last_char[-1] != APR_ASCII_CR)) {
        *last_char = '\0';
        bytes_handled = last_char - *s;
        *read = bytes_handled;
        return APR_EINVAL;
    }

    /* Now NUL-terminate the string at the end of the line;
    /* Now terminate the string at the end of the line;
     * if the last-but-one character is a CR, terminate there */
    if (last_char > *s && last_char[-1] == APR_ASCII_CR) {
        last_char--;
    }
    *last_char = '\0';
    else if (crlf) {
        rv = APR_EINVAL;
        goto cleanup;
    }
    bytes_handled = last_char - *s;

    /* If we're folding, we have more work to do.
@@ -361,7 +354,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
            rv = ap_get_brigade(r->proto_input_filters, bb, AP_MODE_SPECULATIVE,
                                APR_BLOCK_READ, 1);
            if (rv != APR_SUCCESS) {
                return rv;
                goto cleanup;
            }

            if (APR_BRIGADE_EMPTY(bb)) {
@@ -378,7 +371,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
            rv = apr_bucket_read(e, &str, &len, APR_BLOCK_READ);
            if (rv != APR_SUCCESS) {
                apr_brigade_cleanup(bb);
                return rv;
                goto cleanup;
            }

            /* Found one, so call ourselves again to get the next line.
@@ -395,10 +388,8 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
            if (c == APR_ASCII_BLANK || c == APR_ASCII_TAB) {
                /* Do we have enough space? We may be full now. */
                if (bytes_handled >= n) {
                    *read = n;
                    /* ensure this string is terminated */
                    (*s)[n-1] = '\0';
                    return APR_ENOSPC;
                    rv = APR_ENOSPC;
                    goto cleanup;
                }
                else {
                    apr_size_t next_size, next_len;
@@ -411,7 +402,6 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
                        tmp = NULL;
                    }
                    else {
                        /* We're null terminated. */
                        tmp = last_char;
                    }

@@ -420,7 +410,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
                    rv = ap_rgetline_core(&tmp, next_size,
                                          &next_len, r, 0, bb);
                    if (rv != APR_SUCCESS) {
                        return rv;
                        goto cleanup;
                    }

                    if (do_alloc && next_len > 0) {
@@ -434,7 +424,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
                        memcpy(new_buffer, *s, bytes_handled);

                        /* copy the new line, including the trailing null */
                        memcpy(new_buffer + bytes_handled, tmp, next_len + 1);
                        memcpy(new_buffer + bytes_handled, tmp, next_len);
                        *s = new_buffer;
                    }

@@ -447,8 +437,21 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n,
            }
        }
    }

cleanup:
    if (bytes_handled >= n) {
        bytes_handled = n - 1;
    }
    if (*s) {
        /* ensure the string is NUL terminated */
        (*s)[bytes_handled] = '\0';
    }
    *read = bytes_handled;

    if (rv != APR_SUCCESS) {
        return rv;
    }

    /* PR#43039: We shouldn't accept NULL bytes within the line */
    if (strlen(*s) < bytes_handled) {
        return APR_EINVAL;
@@ -487,6 +490,11 @@ AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int flags)
    apr_size_t len;
    apr_bucket_brigade *tmp_bb;

    if (n < 1) {
        /* Can't work since we always NUL terminate */
        return -1;
    }

    tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
    rv = ap_rgetline(&tmp_s, n, &len, r, flags, tmp_bb);
    apr_brigade_destroy(tmp_bb);