Re: com php-src: Fixed bug #61964 (finfo_open with directory cause invalid free): ext/fileinfo/libmagic/apprentice.c ext/fileinfo/tests/bug61964.phpt

I think this also needs an update of the patch file. Or did I overlook that somewhere? Nikita On Fri, May 25, 2012 at 12:55 PM, Stanislav Malyshev <stas@php.net> wrote: > Commit: 1d2f61904987133d542c68cd349cf313d0bef1c8 > Author: Reeze Xia <reeze.xia@gmail.com> Fri, 25 May 2012 18:55:34 +0800 > Parents: c973fef48d6302b9bcec898de8e39d8d7e23adef > Branches: PHP-5.4 master > > Link: > http://git.php.net/?p=php-src.git;a=commitdiff;h=1d2f61904987133d542c68cd349cf313d0bef1c8 > > Log: > Fixed bug #61964 (finfo_open with directory cause invalid free) > > Bugs: > https://bugs.php.net/61964 > > Changed paths: > M ext/fileinfo/libmagic/apprentice.c > A ext/fileinfo/tests/bug61964.phpt > > > Diff: > diff --git a/ext/fileinfo/libmagic/apprentice.c b/ext/fileinfo/libmagic/apprentice.c > index 4a54849..98bde27 100644 > --- a/ext/fileinfo/libmagic/apprentice.c > +++ b/ext/fileinfo/libmagic/apprentice.c > @@ -753,7 +753,7 @@ private int > apprentice_load(struct magic_set *ms, struct magic **magicp, uint32_t *nmagicp, > const char *fn, int action) > { > - int errs = 0; > + int errs = 0, mflen = 0; > struct magic_entry *marray; > uint32_t marraycount, i, mentrycount = 0, starttest; > size_t files = 0, maxfiles = 0; > @@ -782,7 +782,7 @@ apprentice_load(struct magic_set *ms, struct magic **magicp, uint32_t > *nmagicp, > goto out; > } > while ((d = readdir(dir)) != NULL) { > - if (snprintf(mfn, sizeof(mfn), "%s/%s", fn, d->d_name) > < 0) { > + if ((mflen = snprintf(mfn, sizeof(mfn), "%s/%s", fn, > d->d_name)) < 0) { > file_oomem(ms, > strlen(fn) + strlen(d->d_name) + 2); > errs++; > @@ -804,14 +804,14 @@ apprentice_load(struct magic_set *ms, struct magic **magicp, uint32_t > *nmagicp, > goto out; > } > } > - filearr[files++] = mfn; > + filearr[files++] = estrndup(mfn, mflen); > } > closedir(dir); > qsort(filearr, files, sizeof(*filearr), cmpstrp); > for (i = 0; i < files; i++) { > load_1(ms, action, filearr[i], &errs, &marray, > &marraycount); > - free(filearr[i]); > + efree(filearr[i]); > } > free(filearr); > } else > @@ -886,9 +886,14 @@ apprentice_load(struct magic_set *ms, struct magic **magicp, uint32_t > *nmagicp, > mentrycount += marray[i].cont_count; > } > out: > - for (i = 0; i < marraycount; i++) > - efree(marray[i].mp); > - efree(marray); > + for (i = 0; i < marraycount; i++) { > + if (marray[i].mp) { > + efree(marray[i].mp); > + } > + } > + if (marray) { > + efree(marray); > + } > if (errs) { > *magicp = NULL; > *nmagicp = 0; > @@ -1165,6 +1170,9 @@ parse(struct magic_set *ms, struct magic_entry **mentryp, uint32_t > *nmentryp, > return -1; > } > me = &(*mentryp)[*nmentryp - 1]; > + if (me->mp == NULL) { > + return -1; > + } > if (me->cont_count == me->max_count) { > struct magic *nm; > size_t cnt = me->max_count + ALLOC_CHUNK; > @@ -1329,6 +1337,10 @@ parse(struct magic_set *ms, struct magic_entry **mentryp, uint32_t > *nmentryp, > if (m->type == FILE_INVALID) { > if (ms->flags & MAGIC_CHECK) > file_magwarn(ms, "type `%s' invalid", l); > + if (me->mp) { > + efree(me->mp); > + me->mp = NULL; > + } > return -1; > } > > @@ -2219,6 +2231,7 @@ apprentice_map(struct magic_set *ms, struct magic **magicp, uint32_t > *nmagicp, > mm = emalloc((size_t)st.sb.st_size); > if (php_stream_read(stream, mm, (size_t)st.sb.st_size) != (size_t)st.sb.st_size) { > file_badread(ms); > + ret = 1; > goto error1; > } > ret = 1; > diff --git a/ext/fileinfo/tests/bug61964.phpt b/ext/fileinfo/tests/bug61964.phpt > new file mode 100644 > index 0000000..99c8fd2 > --- /dev/null > +++ b/ext/fileinfo/tests/bug61964.phpt > @@ -0,0 +1,69 @@ > +--TEST-- > +Bug #61964 (finfo_open with directory cause invalid free) > +--SKIPIF-- > +<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?> > +--FILE-- > +<?php > + > +$magic_file = dirname(__FILE__) . DIRECTORY_SEPARATOR . 'magic'; > + > +$ret = @finfo_open(FILEINFO_NONE, $magic_file . ".non-exits"); > +var_dump($ret); > + > +$dir = __DIR__ . "/test-folder"; > +@mkdir($dir); > + > +$magic_file_copy = $dir . "/magic.copy"; > +$magic_file_copy2 = $magic_file_copy . "2"; > +copy($magic_file, $magic_file_copy); > +copy($magic_file, $magic_file_copy2); > + > +$ret = finfo_open(FILEINFO_NONE, $dir); > +var_dump($ret); > + > +$ret = @finfo_open(FILEINFO_NONE, $dir); > +var_dump($ret); > + > +$ret = @finfo_open(FILEINFO_NONE, $dir. "/non-exits-dir"); > +var_dump($ret); > + > +// write some test files to test folder > +file_put_contents($dir . "/test1.txt", "string\n> Core\n> Me"); > +file_put_contents($dir . "/test2.txt", "a\nb\n"); > +@mkdir($dir . "/test-inner-folder"); > + > +finfo_open(FILEINFO_NONE, $dir); > +echo "DONE: testing dir with files\n"; > + > +rmdir($dir . "/test-inner-folder"); > +unlink($dir . "/test1.txt"); > +unlink($dir . "/test2.txt"); > + > +unlink($magic_file_copy); > +unlink($magic_file_copy2); > +rmdir($dir); > +?> > +===DONE=== > +--EXPECTF-- > +bool(false) > +resource(%d) of type (file_info) > +resource(%d) of type (file_info) > +bool(false) > + > +Notice: finfo_open(): Warning: offset `string' invalid in %sbug61964.php on line %d > + > +Notice: finfo_open(): Warning: offset ` Core' invalid in %sbug61964.php on line %d > + > +Notice: finfo_open(): Warning: type `Core' invalid in %sbug61964.php on line %d > + > +Notice: finfo_open(): Warning: offset `a' invalid in %sbug61964.php on line %d > + > +Notice: finfo_open(): Warning: type `a' invalid in %sbug61964.php on line %d > + > +Notice: finfo_open(): Warning: offset `b' invalid in %sbug61964.php on line %d > + > +Notice: finfo_open(): Warning: type `b' invalid in %sbug61964.php on line %d > + > +Warning: finfo_open(): Failed to load magic database at '%stest-folder'. in > %sbug61964.php on line %d > +DONE: testing dir with files > +===DONE=== > > > -- > PHP CVS Mailing List (http://www.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php >