httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r795445 - in /httpd/httpd/trunk: include/ap_expr.h modules/filters/mod_include.c server/util_expr.c
Date Sun, 19 Jul 2009 11:04:15 GMT


On 07/19/2009 01:12 AM, niq@apache.org 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

Mime
View raw message