Analysis of PHP's CVE-2016-6289 and CVE-2016-6297

PHP is a programming language that was created in 1995 by Rasmus Lerdorf. And according to W3Techs, it’s dynamically generating content on more than 82% of all websites worldwide. That means hundreds of millions of web servers are vulnerable to the flaws we are describing below.

Last month, FortiGuard discovered two security issues in PHP’s core (CVE-2016-6189) and in PHP’s zip (CVE-2016-6197). These issues affect both the current PHP version 5 and its upcoming version 7. These bugs are located in different part of the code, and feature different functionalities, but they share the same type:

• Integer overflow
• Stack-based buffer overflow

A well-trained eye can identify these kinds of bugs by closely auditing the source code. In the following sections we will describe in more detail these two vulnerabilities.

CVE-2016-6289

This first bug relies on PHP core’s zend_virtual_cwd API. The purpose of this API is to get the current working directory:

<snippet zend/zend_virtual_cwd.c:1243>

CWD_API int virtual_file_ex(cwd_state *state, const char *path, verify_path_func verify_path, int use_realpath) /* {{{ */

{

        int path_length = (int)strlen(path);

        char resolved_path[MAXPATHLEN];

<...>

if (path_length == 0 || path_length >= MAXPATHLEN-1) { /*path_length can be overflow to negative value and passed this check*/

<...>

memcpy(resolved_path, path, path_length + 1);

</snippet>

Notice the path_length variable. It is defined as a signed integer, but never checked for a negative value. It only checks for a value of zero and an oversize value. It then later performs memcpy, and if the value is a negative value it may lead to a crash.

There are two obvious ways to patch this issue. Either redefine path_length­ as an unsigned integer, or validate the value before calling the memory copying function. The first way looks much cleaner and more solid, as it can prevent another integer overflow onto that specific variable. But unfortunately, a negative value can be a valid state in another code path. So the project custodian and I have both agreed that it is best to use the second approach of validating the value.

--- a/Zend/zend_virtual_cwd.c

+++ b/Zend/zend_virtual_cwd.c

@@ -1251,7 +1251,7 @@ CWD_API int virtual_file_ex(cwd_state *state, const char *path, verify_path_func

        int add_slash;

        void *tmp;

-       if (path_length == 0 || path_length >= MAXPATHLEN-1) {

+       if (path_length <= 0 || path_length >= MAXPATHLEN-1) {

 #ifdef ZEND_WIN32

                _set_errno(EINVAL);

 #else

 

CVE-2016-6297

The second bug manifests itself when PHP tries to open the “zip://” handle. When opening a zip stream, php_stream_zip_opener fails to check the path_len, making it vulnerable to an integer overflow. Later, this value will be directly used to calculate the length passed to the memcpy function, which then leads to buffer overflow and memory corruption.

<snippet ext/zip/zip_stream.c:289>

        fragment_len = strlen(fragment);

        if (fragment_len < 1) {

                 return NULL;

        }

        path_len = strlen(path); //path_len can be negative

        if (path_len >= MAXPATHLEN || mode[0] != 'r') {

                 return NULL;

        }

        memcpy(file_dirname, path, path_len - fragment_len); //(path_len - fragment_len) can be controlled

</snippet>

 

 The recommended approach to patch this bug is to simply redefine the data type to fit the purpose. In this circumstance, negative values are never used, so simply changing the data type from integer to size_t (which is an unsigned int) is sufficient.

@@ -263,7 +263,7 @@ php_stream *php_stream_zip_opener(php_stream_wrapper *wrapper,

              zend_string **opened_path, php_stream_context *context STREAMS_DC)

 {

-       int path_len;

+       size_t path_len;

        zend_string *file_basename;

        char file_dirname[MAXPATHLEN];

@@ -271,7 +271,7 @@ php_stream *php_stream_zip_opener(php_stream_wrapper *wrapper,

        struct zip *za;

        struct zip_file *zf = NULL;

        char *fragment;

-       int fragment_len;

+       size_t fragment_len;

Solution

As part of our responsible disclosure process, the detailed information outlined was previously provided to the PHP team, and we worked closely with them to develop a fix as quickly as possible to address these vulnerabilities. I really appreciate the PHP team for their quick replies and prompt patching of these bugs.

PHP has released versions 5.6.24 and 7.0.9 to address these bugs as well as other vulnerabilities. They are available on the PHP project main page at http://www.php.net/downloads.php

As an aside, this is another important example that demonstrates that the only effective way to be protected from this kind of design error issue is to always patch your software.

Keep calm, patch now, and stay safe.

 

-= FortiGuard Lion Team =-