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);