On Mon, 19 Jan 2015 16:28:46 -0600
"William A. Rowe Jr." <[email protected]> wrote:
> On Sun, 18 Jan 2015 23:00:10 -0500
> Eric Covener <[email protected]> wrote:
>
> > On Thu, Oct 30, 2014 at 4:34 AM, <[email protected]> wrote:
> > >
> > > + /* core_dir_config is Directory*, but the requested
> > > file is
> > > + * not a directory, so although the regexp could
> > > match,
> > > + * we skip it. */
> > > + if (entry_core->d_is_directory &&
> > > r->finfo.filetype != APR_DIR) {
> > > + continue;
> > > + }
> > > +
> > > if (ap_regexec(entry_core->r, r->filename, nmatch,
> > > pmatch, 0)) { continue;
> > > }
> >
> > I think this is broken.
>
> You are correct, I don't think it means what the author thought this
> code means.
Indeed, it was nonsense. The fix begins with some code similar to
[attached] but there are a number of shortcuts in the code that would
need to be accounted for by expanding the dir_len at the appropriate
time. The patch is also insufficient in that the r->filename buffer
needs to be at least one extra byte to compensate for the 'maybe a
trailing slash, maybe not' on a final directory component in a file
name.
That said, I'm putting this out there to get folks thinking, I can't
spend much more time on it myself, today.
Cheers,
Bill
Index: request.c
===================================================================
--- request.c (revision 1653666)
+++ request.c (working copy)
@@ -566,6 +566,8 @@
walk_cache_t *cache;
char *entry_dir;
apr_status_t rv;
+ apr_size_t dir_len;
+ char save_ch;
int cached;
/* XXX: Better (faster) tests needed!!!
@@ -1169,8 +1170,11 @@
return r->status = HTTP_FORBIDDEN;
}
+ /* directory-path string length here for DirectoryMatch */
+ dir_len = strlen(r->filename);
+
++seg;
- } while (thisinfo.filetype == APR_DIR);
+ } while (1); /* (thisinfo.filetype == APR_DIR) per above logic */
/* If we have _not_ optimized, this is the time to recover
* the final stat result.
@@ -1191,10 +1195,17 @@
}
}
- /*
- * Now we'll deal with the regexes, note we pick up sec_idx
- * where we left off (we gave up after we hit entry_core->r)
+ /* Now we'll deal with the DirectoryMatch regex's
+ *
+ * First, shorten r->filename to dir_len, plus the trailing
+ * slash when present
*/
+ save_ch = r->filename[dir_len + 1];
+ r->filename[dir_len + 1] = '\0';
+
+ /* Note we pick up sec_idx where we left off
+ * (we gave up above once we hit entry_core->r)
+ */
for (; sec_idx < num_sec; ++sec_idx) {
int nmatch = 0;
@@ -1216,6 +1227,8 @@
pmatch = apr_palloc(rxpool, nmatch*sizeof(ap_regmatch_t));
}
+ /* r->filename here has been truncated to the directory path
+ * component -including- trailing slash
if (ap_regexec(entry_core->r, r->filename, nmatch, pmatch, 0)) {
continue;
}
@@ -1268,6 +1281,8 @@
last_walk->matched = sec_ent[sec_idx];
last_walk->merged = now_merged;
}
+ /* Restore filename now that we have processed DirectoryMatch'es */
+ r->filename[dir_len + 1] = save_ch;
if (rxpool) {
apr_pool_destroy(rxpool);
