Improve simulation mode for session cookies (#259)

Since decrypt_zval doesn't provide a way to tell apart failed and successful decryption when used in simulation mode, we'll have to restore the original value if something goes wrong, because crypto_secretbox_open might modify the value.

2 files changed, 15 insertions, 8 deletions

diff --git a/src/sp_crypt.c b/src/sp_crypt.c
index b6eaa59..525da56 100644
--- a/src/sp_crypt.c
+++ b/src/sp_crypt.c
@@ -91,6 +91,8 @@ int decrypt_zval(zval *pDest, bool simulation, zend_hash_key *hash_key) {
   generate_key(key);
 
   decrypted = ecalloc(ZSTR_LEN(debase64) + crypto_secretbox_ZEROBYTES, 1);
+  char *backup = ecalloc(ZSTR_LEN(debase64), 1);
+  memcpy(backup, ZSTR_VAL(debase64), ZSTR_LEN(debase64));
 
   ret = crypto_secretbox_open(
       decrypted,
@@ -105,19 +107,25 @@ int decrypt_zval(zval *pDest, bool simulation, zend_hash_key *hash_key) {
           "Something went wrong with the decryption of %s. Using the cookie "
           "'as it' instead of decrypting it",
           hash_key ? ZSTR_VAL(hash_key->key) : "the session");
+      memcpy(ZSTR_VAL(debase64), backup, ZSTR_LEN(debase64));
+      efree(backup);
       return ZEND_HASH_APPLY_KEEP;
     } else {
       sp_log_msg("cookie_encryption", SP_LOG_WARN,
                  "Something went wrong with the decryption of %s",
                  hash_key ? ZSTR_VAL(hash_key->key) : "the session");
+      efree(backup);
       return ZEND_HASH_APPLY_REMOVE;
     }
   }
+  efree(backup);
 
   ZVAL_STRINGL(pDest, (char *)(decrypted + crypto_secretbox_ZEROBYTES),
                ZSTR_LEN(debase64) - crypto_secretbox_NONCEBYTES - 1 -
                    crypto_secretbox_ZEROBYTES);
 
+  zend_string_release(decrypted);
+
   return ZEND_HASH_APPLY_KEEP;
 }
 
diff --git a/src/sp_session.c b/src/sp_session.c
index 550b83d..b3db622 100644
--- a/src/sp_session.c
+++ b/src/sp_session.c
@@ -30,19 +30,18 @@ static int sp_hook_s_read(PS_READ_ARGS) {
   const sp_config_session *config_session =
       SNUFFLEUPAGUS_G(config).config_session;
 
-  if (r == SUCCESS && config_session->encrypt && val != NULL && *val != NULL &&
-      ZSTR_LEN(*val)) {
+  if ((NULL == val) || (NULL == *val) || (0 == ZSTR_LEN(*val))) {
+    return r;
+  }
+
+  if (r == SUCCESS && config_session->encrypt) {
     zend_string *orig_val = *val;
     zval val_zval;
     ZVAL_PSTRINGL(&val_zval, ZSTR_VAL(*val), ZSTR_LEN(*val));
 
     int ret = decrypt_zval(&val_zval, config_session->simulation, NULL);
-    if (0 != ret) {
-      if (config_session->simulation) {
-        return ret;
-      } else {
-        zend_bailout();
-      }
+    if (ZEND_HASH_APPLY_KEEP != ret) {
+      zend_bailout();
     }
 
     *val = zend_string_dup(val_zval.value.str, 0);