CVE-2018-15503: Deserialization of Untrusted Data (CWE-502)
value: The unpack implementation in Swoole version 4.0.4 lacks correct size checks in the deserialization process. An attacker can craft a serialized object to exploit this vulnerability and cause a SEGV.
CWE description: The product deserializes untrusted data without sufficiently verifying that the resulting data will be valid.
CVSS v3 base score: 7.5
Published: 2018-08-18T02:29Z
Repository: https://github.com/swoole/swoole-src
Language: C
Files changed: swoole_serialize.c
Commit date: 2018-08-15 19:21:06 +0800
Commit message: add buffer end check
Diff: diff --git a/swoole_serialize.c b/swoole_serialize.c
index fe187d06..a08e2508 100644
--- a/swoole_serialize.c
+++ b/swoole_serialize.c
@@ -50,12 +50,14 @@ static const zend_function_entry swoole_serialize_methods[] = {
 
 zend_class_entry swoole_serialize_ce;
 zend_class_entry *swoole_serialize_class_entry_ptr;
 
 #define SWOOLE_SERI_EOF "EOF"
+#define CHECK_STEP if(buffer>unseri_buffer_end){ php_error_docref(NULL TSRMLS_CC, E_ERROR, "illegal unserialize data"); return NULL;}
 
 static struct _swSeriaG swSeriaG;
+char *unseri_buffer_end = NULL;
 
 void swoole_serialize_init(int module_number TSRMLS_DC)
 {
     SWOOLE_INIT_CLASS_ENTRY(swoole_serialize_ce, "swoole_serialize", "Swoole\\Serialize", swoole_serialize_methods);
     swoole_serialize_class_entry_ptr = zend_register_internal_class(&swoole_serialize_ce TSRMLS_CC);
@@ -111,152 +113,153 @@ static CPINLINE void swoole_check_size(seriaString *str, size_t len)
         }
         str->total = new_size;
     }
 }
 #ifdef __SSE2__
+
 void CPINLINE swoole_mini_memcpy(void *dst, const void *src, size_t len)
 {
     register unsigned char *dd = (unsigned char*) dst + len;
     register const unsigned char *ss = (const unsigned char*) src + len;
     switch (len)
     {
         case 68: *((int*) (dd - 68)) = *((int*) (ss - 68));
-        /* no break */
+            /* no break */
         case 64: *((int*) (dd - 64)) = *((int*) (ss - 64));
-        /* no break */
+            /* no break */
         case 60: *((int*) (dd - 60)) = *((int*) (ss - 60));
-        /* no break */
+            /* no break */
         case 56: *((int*) (dd - 56)) = *((int*) (ss - 56));
-        /* no break */
+            /* no break */
         case 52: *((int*) (dd - 52)) = *((int*) (ss - 52));
-        /* no break */
+            /* no break */
         case 48: *((int*) (dd - 48)) = *((int*) (ss - 48));
-        /* no break */
+            /* no break */
         case 44: *((int*) (dd - 44)) = *((int*) (ss - 44));
-        /* no break */
+            /* no break */
         case 40: *((int*) (dd - 40)) = *((int*) (ss - 40));
-        /* no break */
+            /* no break */
         case 36: *((int*) (dd - 36)) = *((int*) (ss - 36));
-        /* no break */
+            /* no break */
         case 32: *((int*) (dd - 32)) = *((int*) (ss - 32));
-        /* no break */
+            /* no break */
         case 28: *((int*) (dd - 28)) = *((int*) (ss - 28));
-        /* no break */
+            /* no break */
         case 24: *((int*) (dd - 24)) = *((int*) (ss - 24));
-        /* no break */
+            /* no break */
         case 20: *((int*) (dd - 20)) = *((int*) (ss - 20));
-        /* no break */
+            /* no break */
         case 16: *((int*) (dd - 16)) = *((int*) (ss - 16));
-        /* no break */
+            /* no break */
         case 12: *((int*) (dd - 12)) = *((int*) (ss - 12));
-        /* no break */
+            /* no break */
         case 8: *((int*) (dd - 8)) = *((int*) (ss - 8));
-        /* no break */
+            /* no break */
         case 4: *((int*) (dd - 4)) = *((int*) (ss - 4));
             break;
         case 67: *((int*) (dd - 67)) = *((int*) (ss - 67));
-        /* no break */
+            /* no break */
         case 63: *((int*) (dd - 63)) = *((int*) (ss - 63));
-        /* no break */
+            /* no break */
         case 59: *((int*) (dd - 59)) = *((int*) (ss - 59));
-        /* no break */
+            /* no break */
         case 55: *((int*) (dd - 55)) = *((int*) (ss - 55));
-        /* no break */
+            /* no break */
         case 51: *((int*) (dd - 51)) = *((int*) (ss - 51));
-        /* no break */
+            /* no break */
         case 47: *((int*) (dd - 47)) = *((int*) (ss - 47));
-        /* no break */
+            /* no break */
         case 43: *((int*) (dd - 43)) = *((int*) (ss - 43));
-        /* no break */
+            /* no break */
         case 39: *((int*) (dd - 39)) = *((int*) (ss - 39));
-        /* no break */
+            /* no break */
         case 35: *((int*) (dd - 35)) = *((int*) (ss - 35));
-        /* no break */
+            /* no break */
         case 31: *((int*) (dd - 31)) = *((int*) (ss - 31));
-        /* no break */
+            /* no break */
         case 27: *((int*) (dd - 27)) = *((int*) (ss - 27));
-        /* no break */
+            /* no break */
         case 23: *((int*) (dd - 23)) = *((int*) (ss - 23));
-        /* no break */
+            /* no break */
         case 19: *((int*) (dd - 19)) = *((int*) (ss - 19));
-        /* no break */
+            /* no break */
         case 15: *((int*) (dd - 15)) = *((int*) (ss - 15));
-        /* no break */
+            /* no break */
         case 11: *((int*) (dd - 11)) = *((int*) (ss - 11));
-        /* no break */
+            /* no break */
         case 7: *((int*) (dd - 7)) = *((int*) (ss - 7));
             *((int*) (dd - 4)) = *((int*) (ss - 4));
             break;
         case 3: *((short*) (dd - 3)) = *((short*) (ss - 3));
             dd[-1] = ss[-1];
             break;
         case 66: *((int*) (dd - 66)) = *((int*) (ss - 66));
-        /* no break */
+            /* no break */
         case 62: *((int*) (dd - 62)) = *((int*) (ss - 62));
-        /* no break */
+            /* no break */
         case 58: *((int*) (dd - 58)) = *((int*) (ss - 58));
-        /* no break */
+            /* no break */
         case 54: *((int*) (dd - 54)) = *((int*) (ss - 54));
-        /* no break */
+            /* no break */
         case 50: *((int*) (dd - 50)) = *((int*) (ss - 50));
-        /* no break */
+            /* no break */
         case 46: *((int*) (dd - 46)) = *((int*) (ss - 46));
-        /* no break */
+            /* no break */
         case 42: *((int*) (dd - 42)) = *((int*) (ss - 42));
-        /* no break */
+            /* no break */
         case 38: *((int*) (dd - 38)) = *((int*) (ss - 38));
-        /* no break */
+            /* no break */
         case 34: *((int*) (dd - 34)) = *((int*) (ss - 34));
-        /* no break */
+            /* no break */
         case 30: *((int*) (dd - 30)) = *((int*) (ss - 30));
-        /* no break */
+            /* no break */
         case 26: *((int*) (dd - 26)) = *((int*) (ss - 26));
-        /* no break */
+            /* no break */
         case 22: *((int*) (dd - 22)) = *((int*) (ss - 22));
-        /* no break */
+            /* no break */
         case 18: *((int*) (dd - 18)) = *((int*) (ss - 18));
-        /* no break */
+            /* no break */
         case 14: *((int*) (dd - 14)) = *((int*) (ss - 14));
-        /* no break */
+            /* no break */
         case 10: *((int*) (dd - 10)) = *((int*) (ss - 10));
-        /* no break */
+            /* no break */
         case 6: *((int*) (dd - 6)) = *((int*) (ss - 6));
-        /* no break */
+            /* no break */
         case 2: *((short*) (dd - 2)) = *((short*) (ss - 2));
             break;
         case 65: *((int*) (dd - 65)) = *((int*) (ss - 65));
-        /* no break */
+            /* no break */
         case 61: *((int*) (dd - 61)) = *((int*) (ss - 61));
-        /* no break */
+            /* no break */
         case 57: *((int*) (dd - 57)) = *((int*) (ss - 57));
-        /* no break */
+            /* no break */
         case 53: *((int*) (dd - 53)) = *((int*) (ss - 53));
-        /* no break */
+            /* no break */
         case 49: *((int*) (dd - 49)) = *((int*) (ss - 49));
-        /* no break */
+            /* no break */
         case 45: *((int*) (dd - 45)) = *((int*) (ss - 45));
-        /* no break */
+            /* no break */
         case 41: *((int*) (dd - 41)) = *((int*) (ss - 41));
-        /* no break */
+            /* no break */
         case 37: *((int*) (dd - 37)) = *((int*) (ss - 37));
-        /* no break */
+            /* no break */
         case 33: *((int*) (dd - 33)) = *((int*) (ss - 33));
-        /* no break */
+            /* no break */
         case 29: *((int*) (dd - 29)) = *((int*) (ss - 29));
-        /* no break */
+            /* no break */
         case 25: *((int*) (dd - 25)) = *((int*) (ss - 25));
-        /* no break */
+            /* no break */
         case 21: *((int*) (dd - 21)) = *((int*) (ss - 21));
-        /* no break */
+            /* no break */
         case 17: *((int*) (dd - 17)) = *((int*) (ss - 17));
-        /* no break */
+            /* no break */
         case 13: *((int*) (dd - 13)) = *((int*) (ss - 13));
-        /* no break */
+            /* no break */
         case 9: *((int*) (dd - 9)) = *((int*) (ss - 9));
-        /* no break */
+            /* no break */
         case 5: *((int*) (dd - 5)) = *((int*) (ss - 5));
-        /* no break */
+            /* no break */
         case 1: dd[-1] = ss[-1];
             break;
         case 0:
         default: break;
     }
@@ -646,10 +649,11 @@ static CPINLINE void * get_pack_string_len_addr(void ** buffer, size_t *strlen)
 static void* swoole_unserialize_arr(void *buffer, zval *zvalue, uint32_t nNumOfElements, long flag)
 {
     //Initialize zend array
     zend_ulong h, nIndex, max_index = 0;
     uint32_t size = cp_zend_hash_check_size(nNumOfElements);
+    CHECK_STEP;
     if (!size)
     {
         return NULL;
     }
     if (!buffer)
@@ -725,10 +729,11 @@ static void* swoole_unserialize_arr(void *buffer, zval *zvalue, uint32_t nNumOfE
                 else
                 {
                     key_len = *((size_t*) buffer);
                     buffer += sizeof (size_t);
                 }
+                CHECK_STEP;
                 p->key = zend_string_init((char*) buffer, key_len, 0);
                 //           h = zend_inline_hash_func((char*) buffer, key_len);
                 h = zend_inline_hash_func((char*) buffer, key_len);
                 buffer += key_len;
                 p->key->h = p->h = h;
@@ -802,10 +807,11 @@ static void* swoole_unserialize_arr(void *buffer, zval *zvalue, uint32_t nNumOfE
                 else
                 {
                     data_len = *((size_t*) buffer);
                     buffer += sizeof (size_t);
                 }
+                CHECK_STEP;
                 p->val.value.str = zend_string_init((char*) buffer, data_len, 0);
                 buffer += data_len;
             }
             Z_TYPE_INFO(p->val) = IS_STRING_EX;
         }
@@ -830,10 +836,11 @@ static void* swoole_unserialize_arr(void *buffer, zval *zvalue, uint32_t nNumOfE
             Z_TYPE_INFO(p->val) = IS_OBJECT_EX;
         }
 
     }
     ht->nNextFreeElement = max_index;
+    CHECK_STEP;
 
     return buffer;
 
 }
 
@@ -993,11 +1000,11 @@ try_again:
             {
                 zend_array *ht = Z_ARRVAL_P(data);
 
                 if (GC_IS_RECURSIVE(ht))
                 {
-                    ((SBucketType*) (buffer->buffer + p))->data_type = IS_NULL;//reset type null
+                    ((SBucketType*) (buffer->buffer + p))->data_type = IS_NULL; //reset type null
                     php_error_docref(NULL TSRMLS_CC, E_NOTICE, "the array has cycle ref");
                 }
                 else
                 {
                     seria_array_type(ht, buffer, p, buffer->offset);
@@ -1075,10 +1082,11 @@ static CPINLINE void swoole_unserialize_raw(void *buffer, zval *zvalue)
 
     memcpy(&zvalue->value, buffer, sizeof (zend_value));
 }
 
 #if 0
+
 /*
  * null
  */
 static CPINLINE void swoole_unserialize_null(void *buffer, zval *zvalue)
 {
@@ -1248,11 +1256,11 @@ static CPINLINE zend_class_entry* swoole_try_get_ce(zend_string *class_name)
     if ((PG(unserialize_callback_func) == NULL) || (PG(unserialize_callback_func)[0] == '\0'))
     {
         zend_throw_exception_ex(NULL, 0, "can not find class %s", class_name->val TSRMLS_CC);
         return NULL;
     }
-    
+
     zend_string *fname = swoole_string_init(ZEND_STRL(PG(unserialize_callback_func)));
     Z_STR(user_func) = fname;
     Z_TYPE_INFO(user_func) = IS_STRING_EX;
     ZVAL_STR(&args[0], class_name);
 
@@ -1280,28 +1288,30 @@ static CPINLINE zend_class_entry* swoole_try_get_ce(zend_string *class_name)
 static void* swoole_unserialize_object(void *buffer, zval *return_value, zend_uchar bucket_len, zval *args, long flag)
 {
     zval property;
     uint32_t arr_num = 0;
     size_t name_len = *((unsigned short*) buffer);
+    CHECK_STEP;
     if (!name_len)
     {
         php_error_docref(NULL TSRMLS_CC, E_NOTICE, "illegal unserialize data");
         return NULL;
     }
     buffer += 2;
     zend_string *class_name;
-    if (flag == UNSERIALIZE_OBJECT_TO_STDCLASS) 
+    if (flag == UNSERIALIZE_OBJECT_TO_STDCLASS)
     {
         class_name = swoole_string_init(ZEND_STRL("StdClass"));
-    } 
-    else 
+    }
+    else
     {
         class_name = swoole_string_init((char*) buffer, name_len);
     }
     buffer += name_len;
     zend_class_entry *ce = swoole_try_get_ce(class_name);
     swoole_string_release(class_name);
+    CHECK_STEP;
 
     if (!ce)
     {
         return NULL;
     }
@@ -1309,15 +1319,14 @@ static void* swoole_unserialize_object(void *buffer, zval *return_value, zend_uc
     buffer = get_array_real_len(buffer, bucket_len, &arr_num);
     buffer = swoole_unserialize_arr(buffer, &property, arr_num, flag);
 
     object_init_ex(return_value, ce);
 
-    zval *data,*d;
+    zval *data, *d;
     zend_string *key;
     zend_ulong index;
 
-    
     ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL(property), index, key, data)
     {
         const char *prop_name, *tmp;
         size_t prop_len;
         if (key)
@@ -1335,12 +1344,12 @@ static void* swoole_unserialize_object(void *buffer, zval *return_value, zend_uc
             else
             {
                 zend_unmangle_property_name_ex(key, &tmp, &prop_name, &prop_len);
                 zend_update_property(ce, return_value, prop_name, prop_len, data);
             }
-//            zend_hash_update(Z_OBJPROP_P(return_value),key,data);
-//            zend_update_property(ce, return_value, ZSTR_VAL(key), ZSTR_LEN(key), data);
+            //            zend_hash_update(Z_OBJPROP_P(return_value),key,data);
+            //            zend_update_property(ce, return_value, ZSTR_VAL(key), ZSTR_LEN(key), data);
         }
         else
         {
             zend_hash_next_index_insert(Z_OBJPROP_P(return_value), data);
         }
@@ -1386,11 +1395,11 @@ static void* swoole_unserialize_object(void *buffer, zval *return_value, zend_uc
         Z_TYPE_INFO(wakeup) = IS_STRING_EX;
         call_user_function_ex(CG(function_table), return_value, &wakeup, &ret, 0, NULL, 1, NULL);
         swoole_string_release(fname);
         zval_ptr_dtor(&ret);
     }
-
+    CHECK_STEP;
     return buffer;
 
 }
 
 /*
@@ -1484,10 +1493,11 @@ static CPINLINE int swoole_seria_check_eof(void *buffer, size_t len)
  */
 PHPAPI int php_swoole_unserialize(void *buffer, size_t len, zval *return_value, zval *object_args, long flag)
 {
     SBucketType type = *(SBucketType*) (buffer);
     zend_uchar real_type = type.data_type;
+    unseri_buffer_end = buffer + len;
     buffer += sizeof (SBucketType);
     switch (real_type)
     {
         case IS_NULL:
         case IS_TRUE:
@@ -1509,12 +1519,12 @@ PHPAPI int php_swoole_unserialize(void *buffer, size_t len, zval *return_value,
             break;
         case IS_ARRAY:
         {
             if (swoole_seria_check_eof(buffer, len) < 0)
             {
-                  php_error_docref(NULL TSRMLS_CC, E_NOTICE, "detect the error eof");
-                  return SW_FALSE;
+                php_error_docref(NULL TSRMLS_CC, E_NOTICE, "detect the error eof");
+                return SW_FALSE;
             }
             unser_start = buffer - sizeof (SBucketType);
             uint32_t num = 0;
             buffer = get_array_real_len(buffer, type.data_len, &num);
             if (!swoole_unserialize_arr(buffer, return_value, num, flag))
@@ -1524,12 +1534,12 @@ PHPAPI int php_swoole_unserialize(void *buffer, size_t len, zval *return_value,
             break;
         }
         case IS_UNDEF:
             if (swoole_seria_check_eof(buffer, len) < 0)
             {
-                  php_error_docref(NULL TSRMLS_CC, E_NOTICE, "detect the error eof");
-                  return SW_FALSE;
+                php_error_docref(NULL TSRMLS_CC, E_NOTICE, "detect the error eof");
+                return SW_FALSE;
             }
             unser_start = buffer - sizeof (SBucketType);
             if (!swoole_unserialize_object(buffer, return_value, type.data_len, object_args, flag))
             {
                 return SW_FALSE;

Vulnerable code:         /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
        /* no break */
                    ((SBucketType*) (buffer->buffer + p))->data_type = IS_NULL;//reset type null
    
    if (flag == UNSERIALIZE_OBJECT_TO_STDCLASS) 
    } 
    else 
    zval *data,*d;
    
//            zend_hash_update(Z_OBJPROP_P(return_value),key,data);
//            zend_update_property(ce, return_value, ZSTR_VAL(key), ZSTR_LEN(key), data);

                  php_error_docref(NULL TSRMLS_CC, E_NOTICE, "detect the error eof");
                  return SW_FALSE;
                  php_error_docref(NULL TSRMLS_CC, E_NOTICE, "detect the error eof");
                  return SW_FALSE;
Fixed code: #define CHECK_STEP if(buffer>unseri_buffer_end){ php_error_docref(NULL TSRMLS_CC, E_ERROR, "illegal unserialize data"); return NULL;}
char *unseri_buffer_end = NULL;

            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
            /* no break */
    CHECK_STEP;
                CHECK_STEP;
                CHECK_STEP;
    CHECK_STEP;
                    ((SBucketType*) (buffer->buffer + p))->data_type = IS_NULL; //reset type null


    CHECK_STEP;
    if (flag == UNSERIALIZE_OBJECT_TO_STDCLASS)
    }
    else
    CHECK_STEP;
    zval *data, *d;
            //            zend_hash_update(Z_OBJPROP_P(return_value),key,data);
            //            zend_update_property(ce, return_value, ZSTR_VAL(key), ZSTR_LEN(key), data);
    CHECK_STEP;
    unseri_buffer_end = buffer + len;
                php_error_docref(NULL TSRMLS_CC, E_NOTICE, "detect the error eof");
                return SW_FALSE;
                php_error_docref(NULL TSRMLS_CC, E_NOTICE, "detect the error eof");
                return SW_FALSE;