-*- coding: utf-8 -*-
Changes with Apache 2.4.44
+ *) Add support for strict content-length parsing through addition of
+ ap_parse_strict_length() [Yann Ylavic]
+
*) mod_proxy_fcgi: ProxyFCGISetEnvIf unsets variables when expression
evaluates to false. PR64365. [Michael König <mail ikoenig.net>]
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
[ start all new proposals below, under PATCHES PROPOSED. ]
- *) strict content-length parsing
- trunk patch http://svn.apache.org/r1877954
- http://svn.apache.org/r1877955
- http://svn.apache.org/r1879369
- http://svn.apache.org/r1879373
- 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-ap_parse_strict_length.patch
- +1: ylavic, minfrin, jim
- ylavic: the patch drops changes to apreq, which seems to not be in 2.4.
-
*) core: Drop an invalid Last-Modified header value coming
from a (F)CGI script instead of replacing it with Unix epoch.
Warn the users about Last-Modified header value replacements
* 20120211.90 (2.4.42-dev) AP_REG_DEFAULT macro in ap_regex.h
* 20120211.91 (2.4.42-dev) Add ap_is_chunked() in httpd.h
* 20120211.92 (2.4.42-dev) AP_REG_NO_DEFAULT macro in ap_regex.h
+ * 20120211.93 (2.4.44-dev) Add ap_parse_strict_length()
*/
#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 20120211
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 92 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 93 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
AP_DECLARE(char *) ap_append_pid(apr_pool_t *p, const char *string,
const char *delim);
+/**
+ * Parse a length string with decimal characters only, no leading sign nor
+ * trailing character, like Content-Length or (Content-)Range headers.
+ * @param len The parsed length (apr_off_t)
+ * @param str The string to parse
+ * @return 1 (success), 0 (failure)
+ */
+AP_DECLARE(int) ap_parse_strict_length(apr_off_t *len, const char *str);
+
/**
* Parse a given timeout parameter string into an apr_interval_time_t value.
* The unit of the time interval is given as postfix string to the numeric
return APR_SUCCESS;
}
+ /* Set the content length if known.
+ */
+ cl = apr_table_get(r->err_headers_out, "Content-Length");
+ if (cl == NULL) {
+ cl = apr_table_get(r->headers_out, "Content-Length");
+ }
+ if (cl && !ap_parse_strict_length(&size, cl)) {
+ reason = "invalid content length";
+ }
+
if (reason) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00768)
"cache: %s not cached for request %s. Reason: %s",
/* Make it so that we don't execute this path again. */
cache->in_checked = 1;
- /* Set the content length if known.
- */
- cl = apr_table_get(r->err_headers_out, "Content-Length");
- if (cl == NULL) {
- cl = apr_table_get(r->headers_out, "Content-Length");
- }
- if (cl) {
- char *errp;
- if (apr_strtoff(&size, cl, &errp, 10) || *errp || size < 0) {
- cl = NULL; /* parse error, see next 'if' block */
- }
- }
-
if (!cl) {
/* if we don't get the content-length, see if we have all the
* buckets and use their length to calculate the size
* sanity checks.
*/
if (seen_eos) {
- const char *cl_header = apr_table_get(r->headers_out, "Content-Length");
-
if (!dobj->disk_info.header_only) {
+ const char *cl_header;
+ apr_off_t cl;
if (dobj->data.tempfd) {
rv = apr_file_close(dobj->data.tempfd);
apr_pool_destroy(dobj->data.pool);
return APR_EGENERAL;
}
+
if (dobj->file_size < dconf->minfs) {
ap_log_rerror(
APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00734) "URL %s failed the size check "
apr_pool_destroy(dobj->data.pool);
return APR_EGENERAL;
}
- if (cl_header) {
- apr_int64_t cl = apr_atoi64(cl_header);
- if ((errno == 0) && (dobj->file_size != cl)) {
- ap_log_rerror(
- APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00735) "URL %s didn't receive complete response, not caching", h->cache_obj->key);
- /* Remove the intermediate cache file and return non-APR_SUCCESS */
- apr_pool_destroy(dobj->data.pool);
- return APR_EGENERAL;
- }
- }
+ cl_header = apr_table_get(r->headers_out, "Content-Length");
+ if (cl_header && (!ap_parse_strict_length(&cl, cl_header)
+ || cl != dobj->file_size)) {
+ ap_log_rerror(
+ APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00735) "URL %s didn't receive complete response, not caching", h->cache_obj->key);
+ /* Remove the intermediate cache file and return non-APR_SUCCESS */
+ apr_pool_destroy(dobj->data.pool);
+ return APR_EGENERAL;
+ }
}
/* All checks were fine, we're good to go when the commit comes */
/* Was this the final bucket? If yes, perform sanity checks.
*/
if (seen_eos) {
- const char *cl_header = apr_table_get(r->headers_out, "Content-Length");
+ const char *cl_header;
+ apr_off_t cl;
if (r->connection->aborted || r->no_cache) {
ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02380)
sobj->pool = NULL;
return APR_EGENERAL;
}
- if (cl_header) {
- apr_off_t cl;
- char *cl_endp;
- if (apr_strtoff(&cl, cl_header, &cl_endp, 10) != APR_SUCCESS
- || *cl_endp != '\0' || cl != sobj->body_length) {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02381)
- "URL %s didn't receive complete response, not caching",
- h->cache_obj->key);
- apr_pool_destroy(sobj->pool);
- sobj->pool = NULL;
- return APR_EGENERAL;
- }
+
+ cl_header = apr_table_get(r->headers_out, "Content-Length");
+ if (cl_header && (!ap_parse_strict_length(&cl, cl_header)
+ || cl != sobj->body_length)) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02381)
+ "URL %s didn't receive complete response, not caching",
+ h->cache_obj->key);
+ apr_pool_destroy(sobj->pool);
+ sobj->pool = NULL;
+ return APR_EGENERAL;
}
/* All checks were fine, we're good to go when the commit comes */
char *range;
char *dash;
char *slash;
- char *errp;
range_c = apr_table_get(r->headers_in, "content-range");
if (range_c == NULL)
*dash++ = *slash++ = '\0';
/* detect invalid ranges */
- if (apr_strtoff(range_start, range + 6, &errp, 10)
- || *errp || *range_start < 0) {
+ if (!ap_parse_strict_length(range_start, range + 6)) {
return -1;
}
- if (apr_strtoff(range_end, dash, &errp, 10)
- || *errp || *range_end < 0 || *range_end < *range_start) {
+ if (!ap_parse_strict_length(range_end, dash)
+ || *range_end < *range_start) {
return -1;
}
if (*slash != '*') {
apr_off_t dummy;
- if (apr_strtoff(&dummy, slash, &errp, 10)
- || *errp || dummy <= *range_end) {
+ if (!ap_parse_strict_length(&dummy, slash)
+ || dummy <= *range_end) {
return -1;
}
}
r->read_chunked = 1;
}
else if (lenp) {
- const char *pos = lenp;
-
- while (apr_isdigit(*pos) || apr_isspace(*pos)) {
- ++pos;
- }
-
- if (*pos != '\0') {
+ if (!ap_parse_strict_length(&r->remaining, lenp)) {
+ r->remaining = 0;
/* This supplies additional information for the default message. */
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00590)
"Invalid Content-Length %s", lenp);
return HTTP_BAD_REQUEST;
}
-
- r->remaining = apr_atoi64(lenp);
}
if (r->read_chunked || r->remaining > 0) {
if (content_length) {
apr_off_t len, clen;
apr_brigade_length(ctx->bb, 1, &len);
- clen = apr_atoi64(content_length);
- if (clen >= 0 && clen < APR_INT32_MAX) {
+ if (ap_parse_strict_length(&clen, content_length)
+ && clen < APR_INT32_MAX) {
ap_set_content_length(r, len +
apr_base64_encode_len((int)clen) - 1);
}
/* reflect the content length, if present */
if ((content_length = apr_table_get(r->headers_in, "Content-Length"))) {
- apr_off_t offset;
+ apr_off_t clen;
- apr_strtoff(&offset, content_length, NULL, 10);
- ap_set_content_length(r, offset);
+ if (!ap_parse_strict_length(&clen, content_length)) {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10243)
+ "reflector_handler: invalid content-length '%s'",
+ content_length);
+ return HTTP_BAD_REQUEST;
+ }
+ ap_set_content_length(r, clen);
}
/* reflect the content type, if present */
if (!ctx) {
const char *lenp;
- char *endstr = NULL;
request_dir_conf *dconf = ap_get_module_config(f->r->per_dir_config,
&request_module);
if (lenp) {
/* Protects against over/underflow, non-digit chars in the
- * string (excluding leading space) (the endstr checks)
- * and a negative number. */
- if (apr_strtoff(&ctx->remaining, lenp, &endstr, 10)
- || endstr == lenp || *endstr || ctx->remaining < 0) {
-
+ * string, leading plus/minus signs, trailing characters and
+ * a negative number.
+ */
+ if (!ap_parse_strict_length(&ctx->remaining, lenp)) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r, APLOGNO(01411)
- "Invalid Content-Length");
+ "Invalid Content-Length '%s'", lenp);
ap_remove_input_filter(f);
return bail_out_on_error(b, f, HTTP_REQUEST_ENTITY_TOO_LARGE);
*indexes = apr_array_make(r->pool, ranges, sizeof(indexes_t));
while ((cur = ap_getword(r->pool, &range, ','))) {
char *dash;
- char *errp;
apr_off_t number, start, end;
if (!*cur)
if (dash == cur) {
/* In the form "-5" */
- if (apr_strtoff(&number, dash+1, &errp, 10) || *errp) {
+ if (!ap_parse_strict_length(&number, dash+1)) {
return 0;
}
if (number < 1) {
}
else {
*dash++ = '\0';
- if (apr_strtoff(&number, cur, &errp, 10) || *errp) {
+ if (!ap_parse_strict_length(&number, cur)) {
return 0;
}
start = number;
if (*dash) {
- if (apr_strtoff(&number, dash, &errp, 10) || *errp) {
+ if (!ap_parse_strict_length(&number, dash)) {
return 0;
}
end = number;
lenp = NULL;
}
if (lenp) {
- char *endstr;
-
ctx->state = BODY_LENGTH;
/* Protects against over/underflow, non-digit chars in the
- * string (excluding leading space) (the endstr checks)
- * and a negative number. */
- if (apr_strtoff(&ctx->remaining, lenp, &endstr, 10)
- || endstr == lenp || *endstr || ctx->remaining < 0) {
-
+ * string, leading plus/minus signs, trailing characters and
+ * a negative number.
+ */
+ if (!ap_parse_strict_length(&ctx->remaining, lenp)) {
ctx->remaining = 0;
ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f->r, APLOGNO(01587)
"Invalid Content-Length");
r->read_chunked = 1;
}
else if (lenp) {
- char *endstr;
-
- if (apr_strtoff(&r->remaining, lenp, &endstr, 10)
- || *endstr || r->remaining < 0) {
+ if (!ap_parse_strict_length(&r->remaining, lenp)) {
r->remaining = 0;
ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(01594)
- "Invalid Content-Length");
+ "Invalid Content-Length '%s'", lenp);
return HTTP_BAD_REQUEST;
}
}
has_content = 1;
}
else if (!strncmp(buffer, "content-length:", 15)) {
- char *errp;
- apr_off_t number;
+ apr_off_t clen;
body1 = ap_get_token(neg->pool, &body, 0);
- if (apr_strtoff(&number, body1, &errp, 10) != APR_SUCCESS
- || *errp || number < 0) {
+ if (!ap_parse_strict_length(&clen, body1)) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00684)
"Parse error in type map, Content-Length: "
"'%s' in %s is invalid.",
body1, r->filename);
break;
}
- mime_info.bytes = number;
+ mime_info.bytes = clen;
has_content = 1;
}
else if (!strncmp(buffer, "content-language:", 17)) {
/* Did the scheme handler process the request? */
if (access_status != DECLINED) {
const char *cl_a;
- char *end;
apr_off_t cl;
/*
if (access_status != HTTP_BAD_GATEWAY) {
goto cleanup;
}
+
cl_a = apr_table_get(r->headers_in, "Content-Length");
- if (cl_a) {
- apr_strtoff(&cl, cl_a, &end, 10);
+ if (cl_a && (!ap_parse_strict_length(&cl, cl_a)
+ || cl > 0)) {
/*
* The request body is of length > 0. We cannot
* retry with a direct connection since we already
* sent (parts of) the request body to the proxy
* and do not have any longer.
*/
- if (cl > 0) {
- goto cleanup;
- }
+ goto cleanup;
}
/*
* Transfer-Encoding was set as input header, so we had
if (r->main == NULL) {
const char *clp = apr_table_get(r->headers_in, "Content-Length");
- if (clp) {
- char *errp;
- if (apr_strtoff(&len, clp, &errp, 10) || *errp || len < 0) {
- len = 0; /* parse error */
- }
+ if (clp && !ap_parse_strict_length(&len, clp)) {
+ len = -1; /* parse error */
}
}
} else {
/* Get client provided Content-Length header */
content_length = get_content_length(r);
- status = ap_get_brigade(r->input_filters, input_brigade,
- AP_MODE_READBYTES, APR_BLOCK_READ,
- maxsize - AJP_HEADER_SZ);
-
+ if (content_length < 0) {
+ status = APR_EINVAL;
+ }
+ else {
+ status = ap_get_brigade(r->input_filters, input_brigade,
+ AP_MODE_READBYTES, APR_BLOCK_READ,
+ maxsize - AJP_HEADER_SZ);
+ }
if (status != APR_SUCCESS) {
/* We had a failure: Close connection to backend */
conn->close = 1;
}
else if (req->old_cl_val) {
if (r->input_filters == r->proto_input_filters) {
- char *endstr;
- status = apr_strtoff(&req->cl_val, req->old_cl_val, &endstr, 10);
- if (status != APR_SUCCESS || *endstr || req->cl_val < 0) {
+ if (!ap_parse_strict_length(&req->cl_val, req->old_cl_val)) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(01085)
"could not parse request Content-Length (%s)",
req->old_cl_val);
}
if (!r->assbackwards) {
- const char *tenc;
+ const char *tenc, *clen;
ap_get_mime_headers_core(r, tmp_bb);
if (r->status != HTTP_OK) {
goto traceout;
}
+ clen = apr_table_get(r->headers_in, "Content-Length");
+ if (clen) {
+ apr_off_t cl;
+
+ if (!ap_parse_strict_length(&cl, clen)) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(10242)
+ "client sent invalid Content-Length "
+ "(%s): %s", clen, r->uri);
+ r->status = HTTP_BAD_REQUEST;
+ conn->keepalive = AP_CONN_CLOSE;
+ ap_send_error_response(r, 0);
+ ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r);
+ ap_run_log_transaction(r);
+ apr_brigade_destroy(tmp_bb);
+ goto traceout;
+ }
+ }
+
tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
if (tenc) {
- /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
+ /* https://tools.ietf.org/html/rfc7230
* Section 3.3.3.3: "If a Transfer-Encoding header field is
* present in a request and the chunked transfer coding is not
* the final encoding ...; the server MUST respond with the 400
goto traceout;
}
- /* http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23
+ /* https://tools.ietf.org/html/rfc7230
* Section 3.3.3.3: "If a message is received with both a
* Transfer-Encoding and a Content-Length header field, the
* Transfer-Encoding overrides the Content-Length. ... A sender
* MUST remove the received Content-Length field".
*/
- apr_table_unset(r->headers_in, "Content-Length");
+ if (clen) {
+ apr_table_unset(r->headers_in, "Content-Length");
+
+ /* Don't reuse this connection anyway to avoid confusion with
+ * intermediaries and request/reponse spltting.
+ */
+ conn->keepalive = AP_CONN_CLOSE;
+ }
}
}
return APR_SUCCESS;
}
+AP_DECLARE(int) ap_parse_strict_length(apr_off_t *len, const char *str)
+{
+ char *end;
+
+ return (apr_isdigit(*str)
+ && apr_strtoff(len, str, &end, 10) == APR_SUCCESS
+ && *end == '\0');
+}
+
/**
* Determine if a request has a request body or not.
*
AP_DECLARE(int) ap_request_has_body(request_rec *r)
{
apr_off_t cl;
- char *estr;
const char *cls;
- int has_body;
-
- has_body = (!r->header_only
- && (r->kept_body
- || apr_table_get(r->headers_in, "Transfer-Encoding")
- || ( (cls = apr_table_get(r->headers_in, "Content-Length"))
- && (apr_strtoff(&cl, cls, &estr, 10) == APR_SUCCESS)
- && (!*estr)
- && (cl > 0) )
- )
- );
- return has_body;
+
+ return (!r->header_only
+ && (r->kept_body
+ || apr_table_get(r->headers_in, "Transfer-Encoding")
+ || ((cls = apr_table_get(r->headers_in, "Content-Length"))
+ && ap_parse_strict_length(&cl, cls) && cl > 0)));
}
AP_DECLARE_NONSTD(apr_status_t) ap_pool_cleanup_set_null(void *data_)