Patch cleanup for 5.6.5 From df4bf28f9f104ca3ef78ed94b497859f15b004e5 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Sun, 23 Aug 2015 13:27:59 -0700 Subject: [PATCH] Fix bug #70219 (Use after free vulnerability in session deserializer) --- ext/session/session.c | 36 +- ext/session/tests/session_decode_error2.phpt | 518 +++++------------------ ext/session/tests/session_decode_variation3.phpt | 2 +- ext/standard/tests/serialize/bug70219.phpt | 38 ++ ext/standard/var_unserializer.c | 68 +-- ext/standard/var_unserializer.re | 64 +-- 6 files changed, 228 insertions(+), 498 deletions(-) create mode 100644 ext/standard/tests/serialize/bug70219.phpt diff --git a/ext/session/session.c b/ext/session/session.c index 306aba3..0e53c62 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -210,16 +210,18 @@ static char *php_session_encode(int *newlen TSRMLS_DC) /* {{{ */ } /* }}} */ -static void php_session_decode(const char *val, int vallen TSRMLS_DC) /* {{{ */ +static int php_session_decode(const char *val, int vallen TSRMLS_DC) /* {{{ */ { if (!PS(serializer)) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown session.serialize_handler. Failed to decode session object"); - return; + return FAILURE; } if (PS(serializer)->decode(val, vallen TSRMLS_CC) == FAILURE) { php_session_destroy(TSRMLS_C); php_error_docref(NULL TSRMLS_CC, E_WARNING, "Failed to decode session object. Session has been destroyed"); + return FAILURE; } + return SUCCESS; } /* }}} */ @@ -855,8 +857,11 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */ ALLOC_INIT_ZVAL(current); if (php_var_unserialize(¤t, (const unsigned char **) &p, (const unsigned char *) endptr, &var_hash TSRMLS_CC)) { php_set_session_var(name, namelen, current, &var_hash TSRMLS_CC); + } else { + PHP_VAR_UNSERIALIZE_DESTROY(var_hash); + return FAILURE; } - zval_ptr_dtor(¤t); + var_push_dtor_no_addref(&var_hash, ¤t); } PS_ADD_VARL(name, namelen); efree(name); @@ -947,8 +952,13 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */ ALLOC_INIT_ZVAL(current); if (php_var_unserialize(¤t, (const unsigned char **) &q, (const unsigned char *) endptr, &var_hash TSRMLS_CC)) { php_set_session_var(name, namelen, current, &var_hash TSRMLS_CC); + } else { + var_push_dtor_no_addref(&var_hash, ¤t); + efree(name); + PHP_VAR_UNSERIALIZE_DESTROY(var_hash); + return FAILURE; } - zval_ptr_dtor(¤t); + var_push_dtor_no_addref(&var_hash, ¤t); } PS_ADD_VARL(name, namelen); skip: @@ -1922,9 +1932,7 @@ static PHP_FUNCTION(session_decode) return; } - php_session_decode(str, str_len TSRMLS_CC); - - RETURN_TRUE; + RETVAL_BOOL(php_session_decode(str, str_len TSRMLS_CC) == SUCCESS); } /* }}} */ diff --git a/ext/session/tests/session_decode_error2.phpt b/ext/session/tests/session_decode_error2.phpt index 4160f87..515047b 100644 --- a/ext/session/tests/session_decode_error2.phpt +++ b/ext/session/tests/session_decode_error2.phpt @@ -53,563 +53,247 @@ array(0) { } -- Iteration 4 -- -bool(true) -array(1) { - ["foo"]=> - NULL + +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s/session_decode_error2.php on line %d +bool(false) +array(0) { } -- Iteration 5 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 6 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 7 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 8 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 9 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 10 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 11 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 12 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 13 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 14 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 15 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 16 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 17 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 18 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 19 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 20 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 21 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 22 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 23 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 24 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 25 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 26 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 27 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 28 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 29 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 30 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 31 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 32 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 33 -- -bool(true) -array(1) { - ["foo"]=> - NULL +bool(false) +array(0) { } -- Iteration 34 -- -bool(true) -array(1) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 35 -- -bool(true) -array(1) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 36 -- -bool(true) -array(1) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 37 -- -bool(true) -array(1) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 38 -- -bool(true) -array(1) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 39 -- -bool(true) -array(2) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - NULL +bool(false) +array(0) { } -- Iteration 40 -- -bool(true) -array(2) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - NULL +bool(false) +array(0) { } -- Iteration 41 -- -bool(true) -array(2) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - NULL +bool(false) +array(0) { } -- Iteration 42 -- -bool(true) -array(2) { - ["foo"]=> - array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - NULL +bool(false) +array(0) { } -- Iteration 43 -- -bool(true) -array(2) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 44 -- -bool(true) -array(2) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 45 -- -bool(true) -array(2) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 46 -- -bool(true) -array(2) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 47 -- -bool(true) -array(2) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } +bool(false) +array(0) { } -- Iteration 48 -- -bool(true) -array(3) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["blah"]=> - NULL +bool(false) +array(0) { } -- Iteration 49 -- -bool(true) -array(3) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["blah"]=> - NULL +bool(false) +array(0) { } -- Iteration 50 -- -bool(true) -array(3) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["blah"]=> - NULL +bool(false) +array(0) { } -- Iteration 51 -- -bool(true) -array(3) { - ["foo"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["guff"]=> - &array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) - } - ["blah"]=> - NULL +bool(false) +array(0) { } -bool(true) -Done +Warning: session_destroy(): Trying to destroy uninitialized session in %s/session_decode_error2.php on line %d +bool(false) +Done diff --git a/ext/session/tests/session_decode_variation3.phpt b/ext/session/tests/session_decode_variation3.phpt index 4a6f768..0960531 100644 --- a/ext/session/tests/session_decode_variation3.phpt +++ b/ext/session/tests/session_decode_variation3.phpt @@ -49,7 +49,7 @@ array(3) { } Warning: session_decode(): Unknown session.serialize_handler. Failed to decode session object in %s on line %d -bool(true) +bool(false) array(3) { ["foo"]=> int(1234567890) diff --git a/ext/standard/tests/serialize/bug70219.phpt b/ext/standard/tests/serialize/bug70219.phpt new file mode 100644 index 0000000..84a059f --- /dev/null +++ b/ext/standard/tests/serialize/bug70219.phpt @@ -0,0 +1,38 @@ +--TEST-- +Bug #70219 Use after free vulnerability in session deserializer +--FILE-- +data); + } + function unserialize($data) { + session_start(); + session_decode($data); + } +} + +$inner = 'ryat|a:1:{i:0;a:1:{i:1;'; +$exploit = 'a:2:{i:0;C:3:"obj":'.strlen($inner).':{'.$inner.'}i:1;R:4;}'; + +$data = unserialize($exploit); + +for ($i = 0; $i < 5; $i++) { + $v[$i] = 'hi'.$i; +} + +var_dump($data); +?> +--EXPECTF-- +Warning: session_decode(): Failed to decode session object. Session has been destroyed in %s on line %d +array(2) { + [0]=> + object(obj)#%d (1) { + ["data"]=> + NULL + } + [1]=> + array(0) { + } +} diff --git a/ext/standard/var_unserializer.c b/ext/standard/var_unserializer.c index ee0cac4..ffaf680 100644 --- a/ext/standard/var_unserializer.c +++ b/ext/standard/var_unserializer.c @@ -90,7 +90,13 @@ PHPAPI void var_push_dtor(php_unserialize_data_t *var_hashx, zval **rval) PHPAPI void var_push_dtor_no_addref(php_unserialize_data_t *var_hashx, zval **rval) { - var_entries *var_hash = (*var_hashx)->last_dtor; + var_entries *var_hash; + + if (!var_hashx || !*var_hashx) { + return; + } + + var_hash = (*var_hashx)->last_dtor; #if VAR_ENTRIES_DBG fprintf(stderr, "var_push_dtor_no_addref(%ld): %d (%d)\n", var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval), Z_REFCOUNT_PP(rval)); #endif @@ -301,24 +307,20 @@ static inline int process_nested_data(UNSERIALIZE_PARAMETER, HashTable *ht, long ALLOC_INIT_ZVAL(key); if (!php_var_unserialize(&key, p, max, NULL TSRMLS_CC)) { - zval_dtor(key); - FREE_ZVAL(key); + var_push_dtor_no_addref(var_hash, &key); return 0; } if (Z_TYPE_P(key) != IS_LONG && Z_TYPE_P(key) != IS_STRING) { - zval_dtor(key); - FREE_ZVAL(key); + var_push_dtor_no_addref(var_hash, &key); return 0; } ALLOC_INIT_ZVAL(data); if (!php_var_unserialize(&data, p, max, var_hash TSRMLS_CC)) { - zval_dtor(key); - FREE_ZVAL(key); - zval_dtor(data); - FREE_ZVAL(data); + var_push_dtor_no_addref(var_hash, &key); + var_push_dtor_no_addref(var_hash, &data); return 0; } @@ -347,9 +349,7 @@ static inline int process_nested_data(UNSERIALIZE_PARAMETER, HashTable *ht, long sizeof data, NULL); } var_push_dtor(var_hash, &data); - - zval_dtor(key); - FREE_ZVAL(key); + var_push_dtor_no_addref(var_hash, &key); if (elements && *(*p-1) != ';' && *(*p-1) != '}') { (*p)--; diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index abac77c..f02602c 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -89,7 +89,13 @@ PHPAPI void var_push_dtor(php_unserialize_data_t *var_hashx, zval **rval) PHPAPI void var_push_dtor_no_addref(php_unserialize_data_t *var_hashx, zval **rval) { - var_entries *var_hash = (*var_hashx)->last_dtor; + var_entries *var_hash; + + if (!var_hashx || !*var_hashx) { + return; + } + + var_hash = (*var_hashx)->last_dtor; #if VAR_ENTRIES_DBG fprintf(stderr, "var_push_dtor_no_addref(%ld): %d (%d)\n", var_hash?var_hash->used_slots:-1L, Z_TYPE_PP(rval), Z_REFCOUNT_PP(rval)); #endif @@ -307,24 +313,20 @@ static inline int process_nested_data(UNSERIALIZE_PARAMETER, HashTable *ht, long ALLOC_INIT_ZVAL(key); if (!php_var_unserialize(&key, p, max, NULL TSRMLS_CC)) { - zval_dtor(key); - FREE_ZVAL(key); + var_push_dtor_no_addref(var_hash, &key); return 0; } if (Z_TYPE_P(key) != IS_LONG && Z_TYPE_P(key) != IS_STRING) { - zval_dtor(key); - FREE_ZVAL(key); + var_push_dtor_no_addref(var_hash, &key); return 0; } ALLOC_INIT_ZVAL(data); if (!php_var_unserialize(&data, p, max, var_hash TSRMLS_CC)) { - zval_dtor(key); - FREE_ZVAL(key); - zval_dtor(data); - FREE_ZVAL(data); + var_push_dtor_no_addref(var_hash, &key); + var_push_dtor_no_addref(var_hash, &data); return 0; } @@ -353,9 +355,7 @@ static inline int process_nested_data(UNSERIALIZE_PARAMETER, HashTable *ht, long sizeof data, NULL); } var_push_dtor(var_hash, &data); - - zval_dtor(key); - FREE_ZVAL(key); + var_push_dtor_no_addref(var_hash, &key); if (elements && *(*p-1) != ';' && *(*p-1) != '}') { (*p)--; -- 2.1.4 From fc8eff897bd7fe3fed7f6867d2d6a86117a5278d Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Fri, 28 Aug 2015 21:50:21 -0700 Subject: [PATCH] More fixes for bug #70219 --- ext/session/session.c | 7 +++-- ext/standard/tests/serialize/bug70219_1.phpt | 46 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 ext/standard/tests/serialize/bug70219_1.phpt diff --git a/ext/session/session.c b/ext/session/session.c index 247f9b2..f5439ea 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -863,7 +863,10 @@ PS_SERIALIZER_DECODE_FUNC(php_serialize) /* {{{ */ PHP_VAR_UNSERIALIZE_INIT(var_hash); ALLOC_INIT_ZVAL(session_vars); - php_var_unserialize(&session_vars, &val, endptr, &var_hash TSRMLS_CC); + if (php_var_unserialize(&session_vars, &val, endptr, &var_hash TSRMLS_CC)) { + var_push_dtor(&var_hash, &session_vars); + } + PHP_VAR_UNSERIALIZE_DESTROY(var_hash); if (PS(http_session_vars)) { zval_ptr_dtor(&PS(http_session_vars)); @@ -872,7 +875,7 @@ PS_SERIALIZER_DECODE_FUNC(php_serialize) /* {{{ */ array_init(session_vars); } PS(http_session_vars) = session_vars; - ZEND_SET_GLOBAL_VAR_WITH_LENGTH("_SESSION", sizeof("_SESSION"), PS(http_session_vars), 2, 1); + ZEND_SET_GLOBAL_VAR_WITH_LENGTH("_SESSION", sizeof("_SESSION"), PS(http_session_vars), Z_REFCOUNT_P(PS(http_session_vars)) + 1, 1); return SUCCESS; } /* }}} */ diff --git a/ext/standard/tests/serialize/bug70219_1.phpt b/ext/standard/tests/serialize/bug70219_1.phpt new file mode 100644 index 0000000..f9c4c67 --- /dev/null +++ b/ext/standard/tests/serialize/bug70219_1.phpt @@ -0,0 +1,46 @@ +--TEST-- +Bug #70219 Use after free vulnerability in session deserializer +--FILE-- +data); + } + function unserialize($data) { + session_decode($data); + } +} + +$inner = 'r:2;'; +$exploit = 'a:2:{i:0;C:3:"obj":'.strlen($inner).':{'.$inner.'}i:1;C:3:"obj":'.strlen($inner).':{'.$inner.'}}'; + +$data = unserialize($exploit); + +for ($i = 0; $i < 5; $i++) { + $v[$i] = 'hi'.$i; +} + +var_dump($data); +var_dump($_SESSION); +?> +--EXPECTF-- +array(2) { + [0]=> + &object(obj)#%d (1) { + ["data"]=> + NULL + } + [1]=> + object(obj)#%d (1) { + ["data"]=> + NULL + } +} +object(obj)#1 (1) { + ["data"]=> + NULL +} \ No newline at end of file -- 2.1.4