Re: com php-src: Fix stack underflow in phar: ext/phar/phar.c

On 06.08.2018 at 22:43, Anatol Belski wrote: > Commit: b053beee7efb64b8e439fb3639de839e615ba89c > Author: Anatol Belski <ab@php.net> Mon, 6 Aug 2018 22:35:11 +0200 > Parents: 3082600326c2492ef3f24682a1eee0d1b5695c47 > Branches: PHP-7.3 master > > Link: > http://git.php.net/?p=php-src.git;a=commitdiff;h=b053beee7efb64b8e439fb3639de839e615ba89c > > Log: > Fix stack underflow in phar > > The checks can issue reads below and above the temporary buffer. A read > itself doesn't seem dangerous, but the condition result can be > arbitrary. Such reads have to be avoided. Likely this patch should be > backported. > > Changed paths: > M ext/phar/phar.c > > > Diff: > diff --git a/ext/phar/phar.c b/ext/phar/phar.c > index 499eca4..fd716b6 100644 > --- a/ext/phar/phar.c > +++ b/ext/phar/phar.c > @@ -1850,27 +1850,24 @@ static int phar_analyze_path(const char *fname, const char *ext, size_t > ext_len, > /* check for ".phar" in extension */ > static int phar_check_str(const char *fname, const char *ext_str, size_t ext_len, int > executable, int for_create) /* {{{ */ > { > - char test[51]; > const char *pos; > > if (ext_len >= 50) { > return FAILURE; > } > - > if (executable == 1) { > - /* copy "." as well */ > - strlcpy(test, ext_str, ext_len + 1); > - > /* executable phars must contain ".phar" as a valid extension (phar://.pharmy/oops > is invalid) */ > /* (phar://hi/there/.phar/oops is also invalid) */ > - pos = strstr(test, ".phar"); > + pos = strstr(ext_str, ".phar"); > > - if (pos && (*(pos - 1) != '/') > - && (pos += 5) && (*pos == '\0' || *pos == '/' || *pos > == '.')) { > - return phar_analyze_path(fname, ext_str, ext_len, for_create); > - } else { > + if (!pos > + || pos != ext_str && (*(pos - 1) == '/') > + || (ext_len - (pos - ext_str)) < 5 > + || !(pos += 5) > + || !(*pos == '\0' || *pos == '/' || *pos == '.')) { > return FAILURE; > } > + return phar_analyze_path(fname, ext_str, ext_len, for_create); > } > > /* data phars need only contain a single non-"." to be valid */ Sara, what do you think about backporting this to PHP-7.2? -- Christoph M. Becker