From: Jim Jagielski <jim@apache.org>
Date: Tue, 28 Jan 2014 19:40:17 +0000 (+0000)
Subject: Merge r1556206 from trunk:
X-Git-Tag: 2.4.8~188
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=53c4b86e80839734124645c2bb84f8aa20ac4373;p=apache

Merge r1556206 from trunk:

avoid a tight busy loop with memory allocations when the [N] flag
isn't making progress.

If backported, probably increase the hard-coded limit to 32k from 10k.


Submitted by: covener
Reviewed/backported by: jim


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1562174 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/STATUS b/STATUS
index 863409fdcb..2668d1166e 100644
--- a/STATUS
+++ b/STATUS
@@ -98,14 +98,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-   * mod_rewrite: Don't loop forever if the [N] flag isn't making progress.
-     trunk patch http://svn.apache.org/r1556206
-     2.4.x patch: http://people.apache.org/~covener/patches/httpd-2.4.x-rewrite-maxrounds.diff
-                  (incl bump to 32k)
-     +1: covener, jim, trawick
-     trawick: can you clarify here the plan for "If backported, probably increase
-              the hard-coded limit to 32k from 10k."?
-     covener: updated proposal to include bump to 32k
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
diff --git a/docs/manual/rewrite/flags.xml b/docs/manual/rewrite/flags.xml
index 2e0b59f234..ef746841f9 100644
--- a/docs/manual/rewrite/flags.xml
+++ b/docs/manual/rewrite/flags.xml
@@ -392,14 +392,22 @@ certain string or letter repeatedly in a request. The example shown here
 will replace A with B everywhere in a request, and will continue doing
 so until there are no more As to be replaced.
 </p>
-
 <highlight language="config">RewriteRule (.*)A(.*) $1B$2 [N]</highlight>
-
 <p>You can think of this as a <code>while</code> loop: While this
 pattern still matches (i.e., while the URI still contains an
 <code>A</code>), perform this substitution (i.e., replace the
 <code>A</code> with a <code>B</code>).</p>
 
+<p>In 2.4.8 and later, this module returns an error after 32,000 iterations to
+protect against unintended looping.  An alternative maximum number of 
+iterations can be specified by adding to the N flag.  </p>
+<highlight language="config">
+# Be willing to replace 1 character in each pass of the loop
+RewriteRule (.+)[&gt;&lt;;]$ $1 [N=64000]
+# ... or, give up if after 10 loops
+RewriteRule (.+)[&gt;&lt;;]$ $1 [N=10]
+</highlight>
+
 </section>
 
 <section id="flag_nc"><title>NC|nocase</title>
diff --git a/modules/mappers/mod_rewrite.c b/modules/mappers/mod_rewrite.c
index 52b9e1e0f5..9a63bdc1b5 100644
--- a/modules/mappers/mod_rewrite.c
+++ b/modules/mappers/mod_rewrite.c
@@ -231,6 +231,9 @@ static const char* really_last_key = "rewrite_really_last";
 #define subreq_ok(r) (!r->main || \
     (r->main->uri && r->uri && strcmp(r->main->uri, r->uri)))
 
+#ifndef REWRITE_MAX_ROUNDS
+#define REWRITE_MAX_ROUNDS 32000
+#endif
 
 /*
  * +-------------------------------------------------------+
@@ -308,6 +311,7 @@ typedef struct {
     data_item *env;                  /* added environment variables           */
     data_item *cookie;               /* added cookies                         */
     int        skip;                 /* number of next rules to skip          */
+    int        maxrounds;            /* limit on number of loops with N flag  */
 } rewriterule_entry;
 
 typedef struct {
@@ -3498,6 +3502,10 @@ static const char *cmd_rewriterule_setflag(apr_pool_t *p, void *_cfg,
         }
         else if (!*key || !strcasecmp(key, "ext")) {       /* next */
             cfg->flags |= RULEFLAG_NEWROUND;
+            if (val && *val) { 
+                cfg->maxrounds = atoi(val);
+            }
+
         }
         else if (((*key == 'S' || *key == 's') && !key[1])
             || !strcasecmp(key, "osubreq")) {              /* nosubreq */
@@ -3649,6 +3657,7 @@ static const char *cmd_rewriterule(cmd_parms *cmd, void *in_dconf,
     newrule->env = NULL;
     newrule->cookie = NULL;
     newrule->skip   = 0;
+    newrule->maxrounds = REWRITE_MAX_ROUNDS;
     if (a3 != NULL) {
         if ((err = cmd_parseflagfield(cmd->pool, newrule, a3,
                                       cmd_rewriterule_setflag)) != NULL) {
@@ -4192,6 +4201,7 @@ static int apply_rewrite_list(request_rec *r, apr_array_header_t *rewriterules,
     int rc;
     int s;
     rewrite_ctx *ctx;
+    int round = 1;
 
     ctx = apr_palloc(r->pool, sizeof(*ctx));
     ctx->perdir = perdir;
@@ -4280,6 +4290,15 @@ static int apply_rewrite_list(request_rec *r, apr_array_header_t *rewriterules,
              *  the rewriting ruleset again.
              */
             if (p->flags & RULEFLAG_NEWROUND) {
+                if (++round >= p->maxrounds) { 
+                    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02596)
+                                  "RewriteRule '%s' and URI '%s' exceeded "
+                                  "maximum number of rounds (%d) via the [N] flag", 
+                                  p->pattern, r->uri, p->maxrounds);
+
+                    r->status = HTTP_INTERNAL_SERVER_ERROR;
+                    return ACTION_STATUS; 
+                }
                 goto loop;
             }