Skip to content

Commit 90c9c50

Browse files
jmikolajmikola
committed
Merge pull request #250 
2 parents 7d61513 + 3da1aa6 commit 90c9c50

File tree

4 files changed

+122
-16
lines changed

4 files changed

+122
-16
lines changed

‎src/bson.c‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -179,21 +179,9 @@ bool php_phongo_bson_visit_after(const bson_iter_t *iter ARG_UNUSED, const char
179179
}
180180
/* }}} */
181181
#endif
182-
void php_phongo_bson_visit_corrupt(const bson_iter_t *iter ARG_UNUSED, void *data) /* {{{ */
182+
void php_phongo_bson_visit_corrupt(const bson_iter_t *iter ARG_UNUSED, void *data ARG_UNUSED) /* {{{ */
183183
{
184-
#if PHP_VERSION_ID >= 70000
185-
zval *retval = &((php_phongo_bson_state *)data)->zchild;
186-
#else
187-
zval *retval = ((php_phongo_bson_state *)data)->zchild;
188-
#endif
189-
190184
mongoc_log(MONGOC_LOG_LEVEL_TRACE, MONGOC_LOG_DOMAIN, "Corrupt BSON data detected!");
191-
192-
#if PHP_VERSION_ID >= 70000
193-
zval_ptr_dtor(retval);
194-
#else
195-
zval_ptr_dtor(&retval);
196-
#endif
197185
}
198186
/* }}} */
199187
bool php_phongo_bson_visit_double(const bson_iter_t *iter ARG_UNUSED, const char *key, double v_double, void *data) /* {{{ */
@@ -612,7 +600,7 @@ bool php_phongo_bson_visit_document(const bson_iter_t *iter ARG_UNUSED, const ch
612600
array_init(state.zchild);
613601
#endif
614602

615-
if (!bson_iter_visit_all(&child, &php_bson_visitors, &state)) {
603+
if (!bson_iter_visit_all(&child, &php_bson_visitors, &state) && !child.err_off) {
616604
/* If php_phongo_bson_visit_binary() finds an ODM class, it should
617605
* supersede a default type map and named document class. */
618606
if (state.odm && state.map.document_type == PHONGO_TYPEMAP_NONE) {
@@ -662,6 +650,12 @@ bool php_phongo_bson_visit_document(const bson_iter_t *iter ARG_UNUSED, const ch
662650
Z_SET_REFCOUNT_P(state.zchild, 1);
663651
#endif
664652
}
653+
} else {
654+
/* Iteration stopped prematurely due to corruption or a failed
655+
* visitor. Free state.zchild, which we just initialized, and return
656+
* true to stop iteration for our parent context. */
657+
zval_ptr_dtor(&state.zchild);
658+
return true;
665659
}
666660
}
667661

@@ -691,7 +685,7 @@ bool php_phongo_bson_visit_array(const bson_iter_t *iter ARG_UNUSED, const char
691685
array_init(state.zchild);
692686
#endif
693687

694-
if (!bson_iter_visit_all(&child, &php_bson_visitors, &state)) {
688+
if (!bson_iter_visit_all(&child, &php_bson_visitors, &state) && !child.err_off) {
695689

696690
switch(state.map.array_type) {
697691
case PHONGO_TYPEMAP_CLASS: {
@@ -737,6 +731,12 @@ bool php_phongo_bson_visit_array(const bson_iter_t *iter ARG_UNUSED, const char
737731
#endif
738732
break;
739733
}
734+
} else {
735+
/* Iteration stopped prematurely due to corruption or a failed
736+
* visitor. Free state.zchild, which we just initialized, and return
737+
* true to stop iteration for our parent context. */
738+
zval_ptr_dtor(&state.zchild);
739+
return true;
740740
}
741741

742742
}
@@ -1385,7 +1385,15 @@ PHONGO_API int phongo_bson_to_zval_ex(const unsigned char *data, int data_len, p
13851385
#else
13861386
array_init(state->zchild);
13871387
#endif
1388-
bson_iter_visit_all(&iter, &php_bson_visitors, state);
1388+
1389+
if (bson_iter_visit_all(&iter, &php_bson_visitors, state) || iter.err_off) {
1390+
/* Iteration stopped prematurely due to corruption or a failed visitor.
1391+
* While we free the reader, state->zchild should be left as-is, since
1392+
* the calling code may want to zval_ptr_dtor() it. */
1393+
phongo_throw_exception(PHONGO_ERROR_UNEXPECTED_VALUE TSRMLS_CC, "Could not convert BSON document to a PHP variable");
1394+
bson_reader_destroy(reader);
1395+
return 0;
1396+
}
13891397

13901398
/* If php_phongo_bson_visit_binary() finds an ODM class, it should supersede
13911399
* a default type map and named root class. */

‎tests/bson/bson-toPHP_error-003.phpt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
BSON\toPHP(): BSON decoding exceptions for malformed documents
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
5+
--FILE--
6+
<?php
7+
require_once __DIR__ . "/../utils/basic.inc";
8+
9+
$tests = array(
10+
pack('Vx', 4), // Empty document with invalid length (too small)
11+
pack('Vx', 6), // Empty document with invalid length (too large)
12+
);
13+
14+
foreach ($tests as $bson) {
15+
echo throws(function() use ($bson) {
16+
toPHP($bson);
17+
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";
18+
}
19+
20+
?>
21+
===DONE===
22+
<?php exit(0); ?>
23+
--EXPECTF--
24+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
25+
Could not read document from BSON reader
26+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
27+
Could not read document from BSON reader
28+
===DONE===

‎tests/bson/bson-toPHP_error-004.phpt‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
BSON\toPHP(): BSON decoding exceptions for bson_iter_visit_all() failure
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
5+
--FILE--
6+
<?php
7+
require_once __DIR__ . "/../utils/basic.inc";
8+
9+
$tests = array(
10+
// Invalid UTF-8 character in root document's field name
11+
str_replace('INVALID!', "INVALID\xFE", fromPHP(['INVALID!' => 'bar'])),
12+
// Invalid UTF-8 character in embedded document's field name
13+
str_replace('INVALID!', "INVALID\xFE", fromPHP(['foo' => ['INVALID!' => 'bar']])),
14+
// Invalid UTF-8 character in string within array field
15+
str_replace('INVALID!', "INVALID\xFE", fromPHP(['foo' => ['INVALID!']])),
16+
/* Note: we don't use a three-character string in the underflow case, as
17+
* the 4-byte string length and payload (i.e. three characters + null byte)
18+
* coincidentally satisfy the expected size for an 8-byte double. We also
19+
* don't use a four-character string, since its null byte would be
20+
* interpreted as the document terminator. The actual document terminator
21+
* would then remain in the buffer and trigger a "did not exhaust" error.
22+
*/
23+
pack('VCa*xVa*xx', 17, 1, 'foo', 3, 'ab'), // Invalid field type (underflow)
24+
pack('VCa*xVa*xx', 20, 1, 'foo', 6, 'abcde'), // Invalid field type (overflow)
25+
);
26+
27+
foreach ($tests as $bson) {
28+
echo throws(function() use ($bson) {
29+
toPHP($bson);
30+
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";
31+
}
32+
33+
?>
34+
===DONE===
35+
<?php exit(0); ?>
36+
--EXPECTF--
37+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
38+
Could not convert BSON document to a PHP variable
39+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
40+
Could not convert BSON document to a PHP variable
41+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
42+
Could not convert BSON document to a PHP variable
43+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
44+
Could not convert BSON document to a PHP variable
45+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
46+
Could not convert BSON document to a PHP variable
47+
===DONE===

‎tests/bson/bug0531.phpt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
PHPC-531: Segfault due to double free by corrupt BSON visitor
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"?>
5+
--FILE--
6+
<?php
7+
8+
require_once __DIR__ . "/../utils/basic.inc";
9+
10+
$bson = fromPHP(["hello" => "world"]);
11+
$bson[4] = 1;
12+
13+
echo throws(function() use ($bson) {
14+
toPHP($bson);
15+
}, 'MongoDB\Driver\Exception\UnexpectedValueException'), "\n";
16+
17+
?>
18+
===DONE===
19+
<?php exit(0); ?>
20+
--EXPECTF--
21+
OK: Got MongoDB\Driver\Exception\UnexpectedValueException
22+
Could not convert BSON document to a PHP variable
23+
===DONE===

0 commit comments

Comments
 (0)