Skip to content

Commit

Permalink
php: Fix memory corruption for uwsgi_cache_*
Browse files Browse the repository at this point in the history
Ah the joys of variadic arguments in C...

So, when using zend_parse_parameters(), PHP internally loops through the
type specifiers and accordingly uses va_arg() to get the corresponding
argument.

Since the arguments are expected to be pointers to the corresponding
values, the size of them *does* matter, because PHP simply writes to the
corresponding address with a size of size_t.

If we for example pass a pointer to a 32bit integer and PHP writes 64
bits, we have an overflow of 4 bytes.

From README.PARAMETER_PARSING_API in the PHP source tree:

> Please note that since version 7 PHP uses zend_long as integer type
> and zend_string with size_t as length, so make sure you pass
> zend_longs to "l" and size_t to strings length (i.e. for "s" you need
> to pass char * and size_t), not the other way round!
>
> Both mistakes might cause memory corruptions and segfaults:
> 1)
>   char *str;
>   long str_len; /* XXX THIS IS WRONG!! Use size_t instead. */
>   zend_parse_parameters(ZEND_NUM_ARGS(), "s", &str, &str_len)
>
> 2)
>   int num; /* XXX THIS IS WRONG!! Use zend_long instead. */
>   zend_parse_parameters(ZEND_NUM_ARGS(), "l", &num)

To fix this, I changed the types accordingly to use size_t and
zend_long if the PHP major version is >= 7.

Signed-off-by: aszlig <aszlig@nix.build>
  • Loading branch information
aszlig committed Jan 19, 2020
1 parent cff9620 commit f8b4c28
Showing 1 changed file with 24 additions and 16 deletions.
40 changes: 24 additions & 16 deletions plugins/php/php_plugin.c
Expand Up @@ -6,6 +6,14 @@ static sapi_module_struct uwsgi_sapi_module;

static int uwsgi_php_init(void);

#ifdef UWSGI_PHP7
typedef size_t php_strlen_size;
typedef zend_long php_long_size;
#else
typedef int php_strlen_size;
typedef uint64_t php_long_size;
#endif

struct uwsgi_php {
struct uwsgi_string_list *allowed_docroot;
struct uwsgi_string_list *allowed_ext;
Expand Down Expand Up @@ -297,9 +305,9 @@ PHP_FUNCTION(uwsgi_masterpid) {
PHP_FUNCTION(uwsgi_cache_exists) {

char *key = NULL;
int keylen = 0;
php_strlen_size keylen = 0;
char *cache = NULL;
int cachelen = 0;
php_strlen_size cachelen = 0;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|s", &key, &keylen, &cache, &cachelen) == FAILURE) {
RETURN_NULL();
Expand All @@ -315,7 +323,7 @@ PHP_FUNCTION(uwsgi_cache_exists) {
PHP_FUNCTION(uwsgi_cache_clear) {

char *cache = NULL;
int cachelen = 0;
php_strlen_size cachelen = 0;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|s", &cache, &cachelen) == FAILURE) {
RETURN_NULL();
Expand All @@ -330,11 +338,11 @@ PHP_FUNCTION(uwsgi_cache_clear) {


PHP_FUNCTION(uwsgi_cache_del) {

char *key = NULL;
int keylen = 0;
php_strlen_size keylen = 0;
char *cache = NULL;
int cachelen = 0;
php_strlen_size cachelen = 0;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|s", &key, &keylen, &cache, &cachelen) == FAILURE) {
RETURN_NULL();
Expand All @@ -350,9 +358,9 @@ PHP_FUNCTION(uwsgi_cache_del) {
PHP_FUNCTION(uwsgi_cache_get) {

char *key = NULL;
int keylen = 0;
php_strlen_size keylen = 0;
char *cache = NULL;
int cachelen = 0;
php_strlen_size cachelen = 0;
uint64_t valsize;

if (!uwsgi.caches)
Expand All @@ -377,12 +385,12 @@ PHP_FUNCTION(uwsgi_cache_get) {

PHP_FUNCTION(uwsgi_cache_set) {
char *key = NULL;
int keylen;
php_strlen_size keylen = 0;
char *value = NULL;
int vallen;
uint64_t expires = 0;
php_strlen_size vallen = 0;
php_long_size expires = 0;
char *cache = NULL;
int cachelen = 0;
php_strlen_size cachelen = 0;

if (!uwsgi.caches)
RETURN_NULL();
Expand All @@ -400,12 +408,12 @@ PHP_FUNCTION(uwsgi_cache_set) {

PHP_FUNCTION(uwsgi_cache_update) {
char *key = NULL;
int keylen;
php_strlen_size keylen = 0;
char *value = NULL;
int vallen;
uint64_t expires = 0;
php_strlen_size vallen = 0;
php_long_size expires = 0;
char *cache = NULL;
int cachelen = 0;
php_strlen_size cachelen = 0;

if (!uwsgi.caches)
RETURN_NULL();
Expand Down

0 comments on commit f8b4c28

Please sign in to comment.