altarch-user / rpms / httpd

Forked from rpms/httpd 2 years ago
Clone
Blob Blame History Raw
diff --git a/server/protocol.c b/server/protocol.c
index 9e23325..8428129 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -222,6 +222,12 @@ 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 */
+        *read = 0;
+        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
@@ -235,14 +241,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);
@@ -260,7 +267,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) {
@@ -273,17 +280,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? */
@@ -291,7 +289,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 */
@@ -302,7 +300,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);
@@ -326,19 +324,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.
@@ -358,7 +352,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)) {
@@ -375,7 +369,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.
@@ -392,10 +386,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;
@@ -408,7 +400,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;
                     }
 
@@ -417,7 +408,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) {
@@ -431,7 +422,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;
                     }
 
@@ -444,8 +435,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;
@@ -484,6 +488,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);