cdoco/php-jwt

Segmentation fault of php-fpm instance on jwt_decode

dinar opened this issue · 2 comments

dinar commented

php-fpm instance faults with SIGSEGV on invalid token on jwt_decode.
gdb backtrace is below:

Program received signal SIGSEGV, Segmentation fault.
0x00000000005a36ca in zend_hash_str_find ()
(gdb) bt
#0  0x00000000005a36ca in zend_hash_str_find ()
#1  0x000000080603a24a in jwt_verify_body (body=<value optimized out>,
    return_value=0x802a1e1c0) at jwt.c:274
#2  0x000000080603af34 in php_jwt_decode (execute_data=<value optimized out>,
    return_value=0x802a1e1c0) at jwt.c:605
#3  0x00000000006352ad in ZEND_DO_FCALL_BY_NAME_SPEC_RETVAL_USED_HANDLER ()
#4  0x00000000005d4048 in execute_ex ()
#5  0x00000000005d4221 in zend_execute ()
#6  0x000000000058e085 in zend_execute_scripts ()
#7  0x0000000000528802 in php_execute_script ()
#8  0x0000000000670673 in main ()
#9  0x0000000000421885 in _start ()
#10 0x00000008009d8000 in ?? ()
#11 0x0000000000000000 in ?? ()

The following diff:

  • moves jwt_verify_body after jwt_verify function. No need to check body if signature verification is already failed.
  • Add checks in jwt_verify_body for base64 decode and json decode failures.
diff --git a/jwt.c b/jwt.c
index c4dd9b3..2bdce22 100644
--- a/jwt.c
+++ b/jwt.c
@@ -331,10 +331,6 @@ int jwt_verify_body(char *body, zval *return_value)
     time_t curr_time = time((time_t*)NULL);
     zend_string *vs = jwt_b64_url_decode(body);

-    /* decode json to array */
-    php_json_decode_ex(return_value, ZSTR_VAL(vs), ZSTR_LEN(vs), PHP_JSON_OBJECT_AS_ARRAY, 512);
-    zend_string_free(vs);
-
 #define FORMAT_CEX_TIME(t, cex) do {                                                            \
        struct tm *timeinfo;                                                                     \
        char buf[128];                                                                           \
@@ -349,55 +345,68 @@ int jwt_verify_body(char *body, zval *return_value)
         err_msg = msg;                  \
     } while(0);

-    /* set expiration and not before */
-    JWT_G(expiration) = jwt_hash_str_find_long(return_value, "exp");
-    JWT_G(not_before) = jwt_hash_str_find_long(return_value, "nbf");
-    JWT_G(iat) = jwt_hash_str_find_long(return_value, "iat");
-
-    /* expiration */
-    if (JWT_G(expiration) && (curr_time - JWT_G(leeway)) >= JWT_G(expiration))
-        FORMAT_CEX_MSG("Expired token", jwt_expired_signature_cex);
-
-    /* not before */
-    if (JWT_G(not_before) && JWT_G(not_before) > (curr_time + JWT_G(leeway)))
-        FORMAT_CEX_TIME(JWT_G(not_before), jwt_before_valid_cex);
-
-    /* iat */
-    if (JWT_G(iat) && JWT_G(iat) > (curr_time + JWT_G(leeway)))
-        FORMAT_CEX_TIME(JWT_G(iat), jwt_invalid_iat_cex);
-
-    /* iss */
-    if (jwt_verify_claims_str(return_value, "iss", JWT_G(iss)))
-        FORMAT_CEX_MSG("Invalid Issuer", jwt_invalid_issuer_cex);
+    if (!vs) {
+        FORMAT_CEX_MSG("Invalid body", spl_ce_UnexpectedValueException);
+        goto done;
+    }

-    /* jti */
-    if (jwt_verify_claims_str(return_value, "jti", JWT_G(jti)))
-        FORMAT_CEX_MSG("Invalid Jti", jwt_invalid_jti_cex);
+    /* decode json to array */
+    php_json_decode_ex(return_value, ZSTR_VAL(vs), ZSTR_LEN(vs), PHP_JSON_OBJECT_AS_ARRAY, 512);
+    zend_string_free(vs);

-    /* aud */
-    size_t flag = 0;
-    zval *zv_aud = zend_hash_str_find(Z_ARRVAL_P(return_value), "aud", strlen("aud"));
+    if (Z_TYPE(*return_value) == IS_ARRAY) {
+        /* set expiration and not before */
+        JWT_G(expiration) = jwt_hash_str_find_long(return_value, "exp");
+        JWT_G(not_before) = jwt_hash_str_find_long(return_value, "nbf");
+        JWT_G(iat) = jwt_hash_str_find_long(return_value, "iat");
+        /* expiration */
+        if (JWT_G(expiration) && (curr_time - JWT_G(leeway)) >= JWT_G(expiration))
+            FORMAT_CEX_MSG("Expired token", jwt_expired_signature_cex);
+
+        /* not before */
+        if (JWT_G(not_before) && JWT_G(not_before) > (curr_time + JWT_G(leeway)))
+            FORMAT_CEX_TIME(JWT_G(not_before), jwt_before_valid_cex);
+
+        /* iat */
+        if (JWT_G(iat) && JWT_G(iat) > (curr_time + JWT_G(leeway)))
+            FORMAT_CEX_TIME(JWT_G(iat), jwt_invalid_iat_cex);
+
+        /* iss */
+        if (jwt_verify_claims_str(return_value, "iss", JWT_G(iss)))
+            FORMAT_CEX_MSG("Invalid Issuer", jwt_invalid_issuer_cex);
+
+        /* jti */
+        if (jwt_verify_claims_str(return_value, "jti", JWT_G(jti)))
+            FORMAT_CEX_MSG("Invalid Jti", jwt_invalid_jti_cex);
+
+        /* aud */
+        size_t flag = 0;
+        zval *zv_aud = zend_hash_str_find(Z_ARRVAL_P(return_value), "aud", strlen("aud"));
+
+        if (zv_aud && JWT_G(aud)) {
+            switch(Z_TYPE_P(zv_aud)) {
+            case IS_ARRAY:
+                if (jwt_array_equals(Z_ARRVAL_P(JWT_G(aud)), Z_ARRVAL_P(zv_aud))) flag = 1;
+                break;
+            case IS_STRING:
+                if (strcmp(Z_STRVAL_P(JWT_G(aud)), Z_STRVAL_P(zv_aud))) flag = 1;
+                break;
+            default:
+                php_error_docref(NULL, E_WARNING, "Aud type must be string or array");
+                break;
+            }

-    if (zv_aud && JWT_G(aud)) {
-        switch(Z_TYPE_P(zv_aud)) {
-        case IS_ARRAY:
-            if (jwt_array_equals(Z_ARRVAL_P(JWT_G(aud)), Z_ARRVAL_P(zv_aud))) flag = 1;
-            break;
-        case IS_STRING:
-            if (strcmp(Z_STRVAL_P(JWT_G(aud)), Z_STRVAL_P(zv_aud))) flag = 1;
-            break;
-        default:
-            php_error_docref(NULL, E_WARNING, "Aud type must be string or array");
-            break;
+            if (flag) FORMAT_CEX_MSG("Invalid Aud", jwt_invalid_aud_cex);
         }

-        if (flag) FORMAT_CEX_MSG("Invalid Aud", jwt_invalid_aud_cex);
+        /* sub */
+        if (jwt_verify_claims_str(return_value, "sub", JWT_G(sub)))
+            FORMAT_CEX_MSG("Invalid Sub", jwt_invalid_sub_cex);
     }
+    else
+        FORMAT_CEX_MSG("Json decode error", spl_ce_UnexpectedValueException);

-    /* sub */
-    if (jwt_verify_claims_str(return_value, "sub", JWT_G(sub)))
-        FORMAT_CEX_MSG("Invalid Sub", jwt_invalid_sub_cex);
-
+done:
     if (err_msg) {
         zend_throw_exception(ce, err_msg, 0);
         return FAILURE;
@@ -601,11 +610,6 @@ static void php_jwt_decode(INTERNAL_FUNCTION_PARAMETERS) {
         goto decode_done;
     }

-    /* parse body */
-    if (jwt_verify_body(body, return_value) == FAILURE) {
-        goto decode_done;
-    }
-
     /* verify */
     if (jwt->alg == JWT_ALG_NONE) {
         /* done */
@@ -624,6 +628,11 @@ static void php_jwt_decode(INTERNAL_FUNCTION_PARAMETERS) {
         }
     }

+    /* parse body */
+    if (jwt_verify_body(body, return_value) == FAILURE) {
+        goto decode_done;
+    }
+
     smart_str_free(&segments);

 decode_done:
cdoco commented

good job !

But need to move jwt_verify_body behind smart_str_free(&segments);
Otherwise, memory leaks will occur.

-    /* parse body */
-    if (jwt_verify_body(body, return_value) == FAILURE) {
-        goto decode_done;
-    }
-
     /* verify */
     if (jwt->alg == JWT_ALG_NONE) {
         /* done */
@@ -622,9 +623,14 @@ static void php_jwt_decode(INTERNAL_FUNCTION_PARAMETERS) {
         if (jwt_verify(jwt, sig)) {
             zend_throw_exception(jwt_signature_invalid_cex, "Signature verification failed", 0);
         }
+
+        smart_str_free(&segments);
     }

-    smart_str_free(&segments);
+    /* verify body */
+    if (jwt_verify_body(body, return_value) == FAILURE) {
+        goto decode_done;
+    }

 decode_done:
     efree(head);

you can see tests/006.phpt

[Mon Jan  7 16:32:03 2019]  Script:  '/data/php/php-jwt/tests/006.php'
Zend/zend_string.h(134) :  Freeing 0x000000010ac80800 (224 bytes), script=/data/php/php-jwt/tests/006.php
=== Total 1 memory leaks detected ===
dinar commented