From: André Malo Date: Mon, 7 Jul 2003 00:34:10 +0000 (+0000) Subject: well, (kinda) backport LimitInternalRecursion. This prevents X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=843be9882fc4b69d0f52930d00614c38282bd39c;p=thirdparty%2Fapache%2Fhttpd.git well, (kinda) backport LimitInternalRecursion. This prevents the server from crashing if someone configure an infinite loop of internal redirects and subrequests. Default value is 20/20 (subsequent redirects/nested subrequests), 0 means unlimited. The patch works fine on my box, but is required to be tested extensively before the next release. PR: 19753 (and probably more) Obtained from: 2.0 patch Reviewed by: original 2.0 port by Justin and BrianP (?) git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/1.3.x@100468 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/CHANGES b/src/CHANGES index 63b9db884ff..6c56aeb3a46 100644 --- a/src/CHANGES +++ b/src/CHANGES @@ -1,5 +1,11 @@ Changes with Apache 1.3.28 + *) Prevent the server from crashing when entering infinite loops. The + new LimitInternalRecursion directive configures limits of subsequent + internal redirects and nested subrequests, after which the request + will be aborted. PR 19753 (and probably others). + [William Rowe, Jeff Trawick, Jim Jagielski, André Malo] + *) Fix NULL-pointer issue in ab when parsing an incomplete or non-HTTP response. PR 21085. [Glenn Nielsen , André Malo] diff --git a/src/include/ap_mmn.h b/src/include/ap_mmn.h index d10dbc7cec3..19185fbcccb 100644 --- a/src/include/ap_mmn.h +++ b/src/include/ap_mmn.h @@ -242,6 +242,7 @@ * ap_note_cleanups_for_socket_ex(), * ap_note_cleanups_for_file_ex(), * ap_popenf_ex() and ap_psocket_ex(). + * 19990320.15 - ap_is_recursion_limit_exceeded() */ #define MODULE_MAGIC_COOKIE 0x41503133UL /* "AP13" */ @@ -249,7 +250,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 19990320 #endif -#define MODULE_MAGIC_NUMBER_MINOR 14 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 15 /* 0...n */ /* Useful for testing for features. */ #define AP_MODULE_MAGIC_AT_LEAST(major,minor) \ diff --git a/src/include/http_core.h b/src/include/http_core.h index 98efc8935c3..a44498f7ea7 100644 --- a/src/include/http_core.h +++ b/src/include/http_core.h @@ -115,6 +115,12 @@ extern "C" { #define SATISFY_ANY 1 #define SATISFY_NOSPEC 2 +/* default maximum of internal redirects */ +# define AP_DEFAULT_MAX_INTERNAL_REDIRECTS 20 + +/* default maximum subrequest nesting level */ +# define AP_DEFAULT_MAX_SUBREQ_DEPTH 20 + API_EXPORT(int) ap_allow_options (request_rec *); API_EXPORT(int) ap_allow_overrides (request_rec *); API_EXPORT(const char *) ap_default_type (request_rec *); @@ -136,6 +142,12 @@ API_EXPORT(unsigned long) ap_get_limit_req_body(const request_rec *r); API_EXPORT(void) ap_custom_response(request_rec *r, int status, char *string); API_EXPORT(int) ap_exists_config_define(char *name); +/* Check if the current request is beyond the configured max. number of redirects or subrequests + * @param r The current request + * @return true (is exceeded) or false + */ +API_EXPORT(int) ap_is_recursion_limit_exceeded(const request_rec *r); + /* Authentication stuff. This is one of the places where compatibility * with the old config files *really* hurts; they don't discriminate at * all between different authentication schemes, meaning that we need @@ -364,6 +376,11 @@ typedef struct { char *access_name; array_header *sec; array_header *sec_url; + + /* recursion backstopper */ + int recursion_limit_set; /* boolean */ + int redirect_limit; /* maximum number of internal redirects */ + int subreq_limit; /* maximum nesting level of subrequests */ } core_server_config; /* for http_config.c */ diff --git a/src/main/http_core.c b/src/main/http_core.c index 077926c6861..d7703c69d57 100644 --- a/src/main/http_core.c +++ b/src/main/http_core.c @@ -366,7 +366,12 @@ static void *create_core_server_config(pool *a, server_rec *s) conf->ap_document_root = is_virtual ? NULL : DOCUMENT_LOCATION; conf->sec = ap_make_array(a, 40, sizeof(void *)); conf->sec_url = ap_make_array(a, 40, sizeof(void *)); - + + /* recursion stopper */ + conf->redirect_limit = 0; + conf->subreq_limit = 0; + conf->recursion_limit_set = 0; + return (void *)conf; } @@ -387,6 +392,14 @@ static void *merge_core_server_configs(pool *p, void *basev, void *virtv) conf->sec = ap_append_arrays(p, base->sec, virt->sec); conf->sec_url = ap_append_arrays(p, base->sec_url, virt->sec_url); + conf->redirect_limit = virt->recursion_limit_set + ? virt->redirect_limit + : base->redirect_limit; + + conf->subreq_limit = virt->recursion_limit_set + ? virt->subreq_limit + : base->subreq_limit; + return conf; } @@ -3196,6 +3209,134 @@ static const char *set_etag_bits(cmd_parms *cmd, void *mconfig, return NULL; } +static const char *set_recursion_limit(cmd_parms *cmd, void *dummy, + const char *arg1, const char *arg2) +{ + core_server_config *conf = ap_get_module_config(cmd->server->module_config, + &core_module); + int limit = atoi(arg1); + + if (limit < 0) { + return "The recursion limit must be greater than zero."; + } + if (limit && limit < 4) { + ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_NOERRNO, cmd->server, + "Limiting internal redirects to very low numbers may " + "cause normal requests to fail."); + } + + conf->redirect_limit = limit; + + if (arg2) { + limit = atoi(arg2); + + if (limit < 0) { + return "The recursion limit must be greater than zero."; + } + if (limit && limit < 4) { + ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_NOERRNO, cmd->server, + "Limiting the subrequest depth to a very low level may" + " cause normal requests to fail."); + } + } + + conf->subreq_limit = limit; + conf->recursion_limit_set = 1; + + return NULL; +} + +static void log_backtrace(const request_rec *r) +{ + const request_rec *top = r; + + ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, r, + "r->uri = %s", r->uri ? r->uri : "(unexpectedly NULL)"); + + while (top && (top->prev || top->main)) { + if (top->prev) { + top = top->prev; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, r, + "redirected from r->uri = %s", + top->uri ? top->uri : "(unexpectedly NULL)"); + } + + if (!top->prev && top->main) { + top = top->main; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, r, + "subrequested from r->uri = %s", + top->uri ? top->uri : "(unexpectedly NULL)"); + } + } +} + +/* + * check whether redirect limit is reached + */ +API_EXPORT(int) ap_is_recursion_limit_exceeded(const request_rec *r) +{ + core_server_config *conf = ap_get_module_config(r->server->module_config, + &core_module); + const request_rec *top = r; + int redirects = 0, subreqs = 0; + int rlimit = conf->recursion_limit_set + ? conf->redirect_limit + : AP_DEFAULT_MAX_INTERNAL_REDIRECTS; + int slimit = conf->recursion_limit_set + ? conf->subreq_limit + : AP_DEFAULT_MAX_SUBREQ_DEPTH; + + /* fast exit (unlimited) */ + if (!rlimit && !slimit) { + return 0; + } + + while (top->prev || top->main) { + if (top->prev) { + if (rlimit && ++redirects >= rlimit) { + /* uuh, too much. */ + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, r, + "Request exceeded the limit of %d internal " + "redirects due to probable configuration error. " + "Use 'LimitInternalRecursion' to increase the " + "limit if necessary. Use 'LogLevel debug' to get " + "a backtrace.", rlimit); + + /* post backtrace */ + log_backtrace(r); + + /* return failure */ + return 1; + } + + top = top->prev; + } + + if (!top->prev && top->main) { + if (slimit && ++subreqs >= slimit) { + /* uuh, too much. */ + ap_log_rerror(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, r, + "Request exceeded the limit of %d subrequest " + "nesting levels due to probable confguration " + "error. Use 'LimitInternalRecursion' to increase " + "the limit if necessary. Use 'LogLevel debug' to " + "get a backtrace.", slimit); + + /* post backtrace */ + log_backtrace(r); + + /* return failure */ + return 1; + } + + top = top->main; + } + } + + /* recursion state: ok */ + return 0; +} + /* Note --- ErrorDocument will now work from .htaccess files. * The AllowOverride of Fileinfo allows webmasters to turn it off */ @@ -3495,6 +3636,10 @@ static const command_rec core_cmds[] = { { "FileETag", set_etag_bits, NULL, OR_FILEINFO, RAW_ARGS, "Specify components used to construct a file's ETag"}, + +{ "LimitInternalRecursion", set_recursion_limit, NULL, RSRC_CONF, TAKE12, + "maximum recursion depth of internal redirects and subrequests"}, + { NULL } }; diff --git a/src/main/http_request.c b/src/main/http_request.c index 64585ff3a05..4858c0d5fb2 100644 --- a/src/main/http_request.c +++ b/src/main/http_request.c @@ -827,6 +827,13 @@ API_EXPORT(request_rec *) ap_sub_req_method_uri(const char *method, ap_parse_uri(rnew, ap_make_full_path(rnew->pool, udir, new_file)); } + /* We cannot return NULL without violating the API. So just turn this + * subrequest into a 500 to indicate the failure. */ + if (ap_is_recursion_limit_exceeded(r)) { + rnew->status = HTTP_INTERNAL_SERVER_ERROR; + return rnew; + } + res = ap_unescape_url(rnew->uri); if (res) { rnew->status = res; @@ -903,6 +910,13 @@ API_EXPORT(request_rec *) ap_sub_req_lookup_file(const char *new_file, ap_set_sub_req_protocol(rnew, r); fdir = ap_make_dirstr_parent(rnew->pool, r->filename); + /* We cannot return NULL without violating the API. So just turn this + * subrequest into a 500. */ + if (ap_is_recursion_limit_exceeded(r)) { + rnew->status = HTTP_INTERNAL_SERVER_ERROR; + return rnew; + } + /* * Check for a special case... if there are no '/' characters in new_file * at all, then we are looking at a relative lookup in the same @@ -1363,7 +1377,14 @@ static table *rename_original_env(pool *p, table *t) static request_rec *internal_internal_redirect(const char *new_uri, request_rec *r) { int access_status; - request_rec *new = (request_rec *) ap_pcalloc(r->pool, sizeof(request_rec)); + request_rec *new; + + if (ap_is_recursion_limit_exceeded(r)) { + ap_die(HTTP_INTERNAL_SERVER_ERROR, r); + return NULL; + } + + new = (request_rec *) ap_pcalloc(r->pool, sizeof(request_rec)); new->connection = r->connection; new->server = r->server; @@ -1435,7 +1456,10 @@ static request_rec *internal_internal_redirect(const char *new_uri, request_rec API_EXPORT(void) ap_internal_redirect(const char *new_uri, request_rec *r) { request_rec *new = internal_internal_redirect(new_uri, r); - process_request_internal(new); + + if (new) { + process_request_internal(new); + } } /* This function is designed for things like actions or CGI scripts, when @@ -1445,9 +1469,12 @@ API_EXPORT(void) ap_internal_redirect(const char *new_uri, request_rec *r) API_EXPORT(void) ap_internal_redirect_handler(const char *new_uri, request_rec *r) { request_rec *new = internal_internal_redirect(new_uri, r); - if (r->handler) - new->content_type = r->content_type; - process_request_internal(new); + + if (new) { + if (r->handler) + new->content_type = r->content_type; + process_request_internal(new); + } } /*