On 07/19/2009 01:12 AM, [email protected] wrote: > Author: niq > Date: Sat Jul 18 23:12:58 2009 > New Revision: 795445 > > URL: http://svn.apache.org/viewvc?rev=795445&view=rev > Log: > Fix mod_include potential segfault checking backref from unmatched regexp > http://markmail.org/message/jlc7t5edsjujbe37 > Patch by rpluem, lars, niq > > Modified: > httpd/httpd/trunk/include/ap_expr.h > httpd/httpd/trunk/modules/filters/mod_include.c > httpd/httpd/trunk/server/util_expr.c >
> Modified: httpd/httpd/trunk/modules/filters/mod_include.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_include.c?rev=795445&r1=795444&r2=795445&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/filters/mod_include.c (original) > +++ httpd/httpd/trunk/modules/filters/mod_include.c Sat Jul 18 23:12:58 2009 > @@ -605,25 +605,30 @@ > * The choice of returning NULL strings on not-found, > * v.s. empty strings on an empty match is deliberate. > */ > - if (!re) { > + if (!re || !re->have_match) { > ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, > "regex capture $%" APR_SIZE_T_FMT " refers to no regex in > %s", > idx, r->filename); > return NULL; > } > - else { > - if (re->nsub < idx || idx >= AP_MAX_REG_MATCH) { > - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, > - "regex capture $%" APR_SIZE_T_FMT > - " is out of range (last regex was: '%s') in > %s", > - idx, re->rexp, r->filename); > - return NULL; > - } > - > - if (re->match[idx].rm_so < 0 || re->match[idx].rm_eo < 0) { > - return NULL; > - } > + else if (re->match[idx]rm_so == re->match[idx].rm_eo) { > + return NULL; > + } > + else if (re->match[idx].rm_so < 0 || re->match[idx].rm_eo < 0) { > + /* I don't think this can happen if have_match is true. > + * But let's not risk a regression by dropping this > + */ > + return NULL; > + } > + else if (re->nsub < idx || idx >= AP_MAX_REG_MATCH) { IMHO this check should be the first (as in the old code) as it ensures that idx is of correct range. So it should be moved before re->match[idx].rm_so == re->match[idx].rm_eo) > + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, > + "regex capture $%" APR_SIZE_T_FMT > + " is out of range (last regex was: '%s') in %s", > + idx, re->rexp, r->filename); > + return NULL; > + } > > + else { > val = apr_pstrmemdup(ctx->dpool, re->source + > re->match[idx].rm_so, > re->match[idx].rm_eo - > re->match[idx].rm_so); As apr_pstrmemdup does return '\0' instead of NULL when re->match[idx].rm_so == re->match[idx].rm_eo we change the behaviour by doing the re->match[idx].rm_so == re->match[idx].rm_eo check above. Regards Rüdiger
