Re: A potential patch for Bug#60668

Hello, On Wed, Jan 25, 2012 at 5:43 PM, Kiyoto Tamura <me@ktamura.com> wrote: > vrana has raise a good point in a potentially dangerous behavior with ini_set() in > https://bugs.php.net/bug.php?id=60668. > > Here is my proposed patch. Feedback is appreciated. Thanks! > > Kiyoto Tamura > > diff --git a/Zend/zend_ini.c b/Zend/zend_ini.c > index a7ec5d7..89b1287 100644 > --- a/Zend/zend_ini.c > +++ b/Zend/zend_ini.c > @@ -83,6 +83,23 @@ static int zend_restore_ini_entry_wrapper(zend_ini_entry **ini_entry > TSRMLS_DC) >  } >  /* }}} */ > > +static uint zend_trim_after_carriage_return(char *value, uint value_length) /* {{{ */ > +{ > +    uint ii; > +    char prev_c = '\0', curr_c; > +    for (ii = 0; ii < value_length; ++ii) { > +        curr_c = *value; > +        if (prev_c == '\r' && curr_c == '\n') { > +            return ii - 1; > +        } > +        prev_c = curr_c; > +        ++value; > +    } > + > +    return value_length; > +} > +/* }}} */ > + To comment specifically on the patch itself; I am not sure you need a new zend func here, I think it would be more convenient (and safer) to use php_trim in standard/string.c. Although I must say looking at the bug, I am not sure this is exactly something that should be "fixed". It could be a BC break and in my opinion is just a demonstration of failure to properly sanitize user input as opposed to a security flaw in php design itself. -Chris