Implement anti csrf measures

This is done by using the "samesite" cookie attribute.

doc/source/config.rst

@@ -115,6 +115,26 @@ It can either be ``enabled`` or ``disabled``.
   sp.auto_cookie_secure.enable();
   sp.auto_cookie_secure.disable();
 
+cookie_samesite
+^^^^^^^^^^^^^^^^
+ * `default: disabled`
+
+``samesite`` will add the `samesite <https://tools.ietf.org/html/draft-west-first-party-cookies-07>`_
+attribute to cookies. It `prevents CSRF <https://www.owasp.org/index.php/SameSite>`_
+but is not implemented by `all web browsers <https://caniuse.com/#search=samesite>`_ yet.
+
+It can either be set to ``strict`` or ``lax``:
+
+- The ``lax`` attribute prevents cookies from being sent cross-domain for
+  "dangerous" methods, like ``POST``, ``PUT`` or ``DELETE``.
+
+- The ``strict`` one prevents any cookies from beind sent cross-domain.
+
+::
+
+  sp.cookie.name("cookie1").samesite("lax");
+  sp.cookie.name("cookie2").samesite("strict");;
+
 .. _cookie-encryption_config:
 
 cookie_encryption
@@ -137,8 +157,8 @@ It can either be ``enabled`` or ``disabled`` and can be used in ``simulation`` m
 
 ::
 
-  sp.cookie_encryption.cookie("my_cookie_name");
-  sp.cookie_encryption.cookie("another_cookie_name");
+  sp.cookie.name("my_cookie_name").encrypt();
+  sp.cookie.name("another_cookie_name").encrypt();
 
 Choosing the proper environment variable
 """"""""""""""""""""""""""""""""""""""""

doc/source/features.rst

@@ -321,6 +321,20 @@ would be to use a different user to run PHP than for administrating the website,
 and using this feature to lock this up.
 
 
+Protection against cross site request forgery
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Cross-site request forgery, sometimes abbreviated as *CSRF*,
+is when unauthorised commands are issued from a user that the application trusts.
+For example, if a user is authenticated on a banking website,
+an other site might present something like 
+``<img src="http://mybank.com/transfer?from=user&to=attack&amount=1337EUR">``,
+effectivement transfering money from the user's account to the attacker one.
+
+Snuffleupagus can prevent this (in `supported browsers <https://caniuse.com/#search=samesite>`__)
+by setting the `samesite <https://tools.ietf.org/html/draft-west-first-party-cookies-07>`__
+attribute on cookies.
+
 
 Dumping capabilities
 ^^^^^^^^^^^^^^^^^^^^

src/snuffleupagus.c

@@ -71,14 +71,13 @@ PHP_GINIT_FUNCTION(snuffleupagus) {
   SP_INIT(snuffleupagus_globals->config.config_upload_validation);
   SP_INIT(snuffleupagus_globals->config.config_disabled_functions);
   SP_INIT(snuffleupagus_globals->config.config_disabled_functions_ret);
-  SP_INIT(snuffleupagus_globals->config.config_cookie_encryption);
+  SP_INIT(snuffleupagus_globals->config.config_cookie);
   SP_INIT(snuffleupagus_globals->config.config_disabled_constructs);
 
   snuffleupagus_globals->config.config_disabled_constructs->construct_include = sp_list_new();
   snuffleupagus_globals->config.config_disabled_functions->disabled_functions = sp_list_new();
   snuffleupagus_globals->config.config_disabled_functions_ret->disabled_functions = sp_list_new();
-
-  SP_INIT_HT(snuffleupagus_globals->config.config_cookie_encryption->names);
+  SP_INIT_HT(snuffleupagus_globals->config.config_cookie->cookies);
 
 #undef SP_INIT
 #undef SP_INIT_HT
@@ -96,7 +95,7 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) {
   pefree(SNUFFLEUPAGUS_G(F), 1);
 
   FREE_HT(disabled_functions_hook);
-  FREE_HT(config.config_cookie_encryption->names);
+  FREE_HT(config.config_cookie->cookies);
 
 #undef FREE_HT
 
@@ -108,7 +107,6 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) {
   pefree(SNUFFLEUPAGUS_G(config.config_snuffleupagus), 1);
   pefree(SNUFFLEUPAGUS_G(config.config_disable_xxe), 1);
   pefree(SNUFFLEUPAGUS_G(config.config_upload_validation), 1);
-  pefree(SNUFFLEUPAGUS_G(config.config_cookie_encryption), 1);
 
 #define FREE_LST(L) \
   do { \
@@ -126,6 +124,7 @@ PHP_MSHUTDOWN_FUNCTION(snuffleupagus) {
   pefree(SNUFFLEUPAGUS_G(config.config_disabled_functions), 1);
   pefree(SNUFFLEUPAGUS_G(config.config_disabled_functions_ret), 1);
   pefree(SNUFFLEUPAGUS_G(config.config_disabled_constructs), 1);
+  pefree(SNUFFLEUPAGUS_G(config.config_cookie), 1);
 
   UNREGISTER_INI_ENTRIES();
 
@@ -137,9 +136,9 @@ PHP_RINIT_FUNCTION(snuffleupagus) {
   ZEND_TSRMLS_CACHE_UPDATE();
 #endif
   if (NULL != SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key) {
-    if (NULL != SNUFFLEUPAGUS_G(config).config_cookie_encryption->names) {
+    if (NULL != SNUFFLEUPAGUS_G(config).config_cookie->cookies) {
       zend_hash_apply_with_arguments(
-          Z_ARRVAL(PG(http_globals)[TRACK_VARS_COOKIE]), decrypt_cookie, 0);
+	  Z_ARRVAL(PG(http_globals)[TRACK_VARS_COOKIE]), decrypt_cookie, 0);
     }
   }
   return SUCCESS;
@@ -190,8 +189,8 @@ static PHP_INI_MH(OnUpdateConfiguration) {
     if (SNUFFLEUPAGUS_G(config).config_unserialize->enable) {
       hook_serialize();
     }
-    hook_cookies();
   }
+  hook_cookies();
 
   if (true == SNUFFLEUPAGUS_G(config).config_global_strict->enable) {
     if (!zend_get_extension(PHP_SNUFFLEUPAGUS_EXTNAME)) {

src/sp_config.c

@@ -15,7 +15,7 @@ sp_config_tokens const sp_func[] = {
     {.func = parse_readonly_exec, .token = SP_TOKEN_READONLY_EXEC},
     {.func = parse_global_strict, .token = SP_TOKEN_GLOBAL_STRICT},
     {.func = parse_upload_validation, .token = SP_TOKEN_UPLOAD_VALIDATION},
-    {.func = parse_cookie_encryption, .token = SP_TOKEN_COOKIE_ENCRYPTION},
+    {.func = parse_cookie, .token = SP_TOKEN_COOKIE_ENCRYPTION},
     {.func = parse_global, .token = SP_TOKEN_GLOBAL},
     {.func = parse_auto_cookie_secure, .token = SP_TOKEN_AUTO_COOKIE_SECURE},
     {.func = parse_disable_xxe, .token = SP_TOKEN_DISABLE_XXE},

src/sp_config.h

@@ -55,7 +55,12 @@ typedef struct { bool enable; } sp_config_auto_cookie_secure;
 
 typedef struct { bool enable; } sp_config_disable_xxe;
 
-typedef struct { HashTable *names; } sp_config_cookie_encryption;
+enum samesite_type {strict=1, lax=2};
+
+typedef struct {
+  enum samesite_type samesite;
+  bool encrypt;
+} sp_cookie;
 
 typedef struct {
   bool enable;
@@ -104,6 +109,10 @@ typedef struct {
   sp_node_t *disabled_functions;  // list of sp_disabled_function
 } sp_config_disabled_functions;
 
+typedef struct {
+  HashTable *cookies;  // HashTable of sp_cookie
+} sp_config_cookie;
+
 typedef struct {
   sp_node_t *construct_include; // list of rules for `(include|require)_(once)?`
   sp_node_t *construct_echo;
@@ -122,7 +131,7 @@ typedef struct {
   sp_config_disabled_functions *config_disabled_functions_ret;
   sp_config_readonly_exec *config_readonly_exec;
   sp_config_upload_validation *config_upload_validation;
-  sp_config_cookie_encryption *config_cookie_encryption;
+  sp_config_cookie *config_cookie;
   sp_config_global *config_snuffleupagus;
   sp_config_auto_cookie_secure *config_auto_cookie_secure;
   sp_config_global_strict *config_global_strict;
@@ -144,7 +153,7 @@ typedef struct {
 #define SP_TOKEN_BASE "sp"
 
 #define SP_TOKEN_AUTO_COOKIE_SECURE ".auto_cookie_secure"
-#define SP_TOKEN_COOKIE_ENCRYPTION ".cookie_encryption"
+#define SP_TOKEN_COOKIE_ENCRYPTION ".cookie"
 #define SP_TOKEN_DISABLE_FUNC ".disable_function"
 #define SP_TOKEN_GLOBAL ".global"
 #define SP_TOKEN_GLOBAL_STRICT ".global_strict"
@@ -187,7 +196,13 @@ typedef struct {
 #define SP_TOKEN_LINE_NUMBER ".line("
 
 // cookies encryption
-#define SP_TOKEN_NAME ".cookie("
+#define SP_TOKEN_NAME ".name("
+
+// cookies samesite
+#define SP_TOKEN_SAMESITE ".samesite("
+#define SP_TOKEN_ENCRYPT ".encrypt("
+#define SP_TOKEN_SAMESITE_LAX "Lax"
+#define SP_TOKEN_SAMESITE_STRICT "Strict"
 
 // Global configuration options
 #define SP_TOKEN_ENCRYPTION_KEY ".secret_key("

src/sp_config_keywords.c

@@ -105,38 +105,59 @@ int parse_global(char *line) {
   return parse_keywords(sp_config_funcs_global, line);
 }
 
-int parse_cookie_encryption(char *line) {
+int parse_cookie(char *line) {
   int ret = 0;
-  char *name = NULL;
+  char *samesite = NULL, *name = NULL;
+  sp_cookie *cookie = pecalloc(sizeof(sp_cookie), 1, 1);
+  zend_string *zend_name;
 
   sp_config_functions sp_config_funcs_cookie_encryption[] = {
       {parse_str, SP_TOKEN_NAME, &name},
+      {parse_str, SP_TOKEN_SAMESITE, &samesite},
+      {parse_empty, SP_TOKEN_ENCRYPT, &cookie->encrypt},
       {0}};
 
   ret = parse_keywords(sp_config_funcs_cookie_encryption, line);
   if (0 != ret) {
     return ret;
   }
 
-  if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->cookies_env_var)) {
-    sp_log_err("config", "You're trying to use the cookie encryption feature"
-      "on line %zu without having set the `.cookie_env_var` option in"
-      "`sp.global`: please set it first.", sp_line_no);
-    return -1;
-  } else if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key)) {
-    sp_log_err("config", "You're trying to use the cookie encryption feature"
-      "on line %zu without having set the `.encryption_key` option in"
-      "`sp.global`: please set it first.", sp_line_no);
+  if (cookie->encrypt) {
+    if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->cookies_env_var)) {
+      sp_log_err("config", "You're trying to use the cookie encryption feature"
+	"on line %zu without having set the `.cookie_env_var` option in"
+	"`sp.global`: please set it first.", sp_line_no);
+      return -1;
+    } else if (0 == (SNUFFLEUPAGUS_G(config).config_snuffleupagus->encryption_key)) {
+      sp_log_err("config", "You're trying to use the cookie encryption feature"
+	"on line %zu without having set the `.encryption_key` option in"
+	"`sp.global`: please set it first.", sp_line_no);
+      return -1;
+    }
+  } else if (!samesite) {
+    sp_log_err("config", "You must specify a at least one action to a cookie on line "
+      "%zu.", sp_line_no);
     return -1;
-  } else if (0 == strlen(name)) {
-    sp_log_err("config", "You must specify a cookie name to encrypt on line "
+  }
+  if (0 == strlen(name)) {
+    sp_log_err("config", "You must specify a cookie name on line "
       "%zu.", sp_line_no);
     return -1;
   }
+  if (samesite) {
+    if (0 == strcasecmp(samesite, SP_TOKEN_SAMESITE_LAX)) {
+      cookie->samesite = lax;
+    } else if (0 == strcasecmp(samesite, SP_TOKEN_SAMESITE_STRICT)) {
+      cookie->samesite = strict;
+    } else {
+      sp_log_err("config", "%s is an invalid value to samesite (expected %s or %s) on line "
+	"%zu.", samesite, SP_TOKEN_SAMESITE_LAX, SP_TOKEN_SAMESITE_STRICT, sp_line_no);
+      return -1;
+    }
+  }
 
-  zend_hash_str_add_empty_element(
-      SNUFFLEUPAGUS_G(config).config_cookie_encryption->names, name,
-      strlen(name));
+  zend_name = zend_string_init(name, strlen(name), 1);
+  zend_hash_add_ptr(SNUFFLEUPAGUS_G(config).config_cookie->cookies, zend_name, cookie);
 
   return SUCCESS;
 }

src/sp_config_keywords.h

@@ -7,10 +7,10 @@ int parse_disable_xxe(char *line);
 int parse_auto_cookie_secure(char *line);
 int parse_global_strict(char *line);
 int parse_global(char *line) ;
-int parse_cookie_encryption(char *line);
+int parse_cookie(char *line);
 int parse_unserialize(char *line) ;
 int parse_readonly_exec(char *line);
 int parse_disabled_functions(char *line) ;
 int parse_upload_validation(char *line);
 
-#endif // __SP_CONFIG_KEYWORDS_H
+#endif // __SP_CONFIG_KEYWORDS_H

src/sp_cookie_encryption.c

@@ -45,12 +45,12 @@ int decrypt_cookie(zval *pDest, int num_args, va_list args,
   size_t value_len;
   zend_string *debase64;
   unsigned char *decrypted;
+  sp_cookie *cookie = zend_hash_find_ptr(SNUFFLEUPAGUS_G(config).config_cookie->cookies,
+					 hash_key->key);
   int ret = 0;
 
   /* If the cookie isn't in the conf, it shouldn't be encrypted. */
-  if (0 ==
-      zend_hash_exists(SNUFFLEUPAGUS_G(config).config_cookie_encryption->names,
-                       hash_key->key)) {
+  if (!cookie || !cookie->encrypt) {
     return ZEND_HASH_APPLY_KEEP;
   }
 
@@ -135,11 +135,13 @@ static zend_string *encrypt_data(char *data, unsigned long long data_len) {
 
 PHP_FUNCTION(sp_setcookie) {
   zval params[7] = { 0 };
-  zend_string *name = NULL, *value = NULL, *path = NULL, *domain = NULL;
+  zend_string *name = NULL, *value = NULL, *path = NULL, *domain = NULL, *samesite = NULL;
   zend_long expires = 0;
   zend_bool secure = 0, httponly = 0;
   zval ret_val;
+  const sp_cookie *cookie_node = NULL;
   zval func_name;
+  char *cookie_samesite;
 
 
   // LCOV_EXCL_BR_START
@@ -167,17 +169,18 @@ PHP_FUNCTION(sp_setcookie) {
     }
   }
 
+  cookie_node =
+    zend_hash_find_ptr(SNUFFLEUPAGUS_G(config).config_cookie->cookies, name);
+
   /* If the cookie's value is encrypted, it won't be usable by
    * javascript anyway.
    */
-  if (zend_hash_exists(SNUFFLEUPAGUS_G(config).config_cookie_encryption->names,
-                       name) > 0) {
+  if (cookie_node && cookie_node->encrypt) {
     httponly = 1;
   }
 
   /* Shall we encrypt the cookie's value? */
-  if (zend_hash_exists(SNUFFLEUPAGUS_G(config).config_cookie_encryption->names,
-                       name) > 0 && value) {
+  if (httponly && value) {
     zend_string *encrypted_data = encrypt_data(value->val, value->len);
     ZVAL_STR_COPY(&params[1], encrypted_data);
     zend_string_release(encrypted_data);
@@ -188,9 +191,6 @@ PHP_FUNCTION(sp_setcookie) {
   ZVAL_STRING(&func_name, "setcookie");
   ZVAL_STR_COPY(&params[0], name);
   ZVAL_LONG(&params[2], expires);
-  if (path) {
-    ZVAL_STR_COPY(&params[3], path);
-  }
   if (domain) {
     ZVAL_STR_COPY(&params[4], domain);
   }
@@ -201,6 +201,23 @@ PHP_FUNCTION(sp_setcookie) {
     ZVAL_LONG(&params[6], httponly);
   }
 
+  /* param[3](path) is concatenated to path= and is not filtered, we can inject
+  the samesite parameter here */
+  if (cookie_node && cookie_node->samesite) {
+    if (!path) {
+      path = zend_string_init("", 0, 0);
+    }
+    cookie_samesite = (cookie_node->samesite == lax) ? SAMESITE_COOKIE_FORMAT SP_TOKEN_SAMESITE_LAX
+      : SAMESITE_COOKIE_FORMAT SP_TOKEN_SAMESITE_STRICT;
+    /* Concatenating everything, as is in PHP internals */
+    samesite = zend_string_extend(path, ZSTR_LEN(path) + strlen(cookie_samesite) + 1, 0);
+    memcpy(ZSTR_VAL(samesite) + ZSTR_LEN(path), cookie_samesite, strlen(cookie_samesite) + 1);
+    ZVAL_STR_COPY(&params[3], samesite);
+    zend_string_release(path);
+  } else if (path) {
+    ZVAL_STR_COPY(&params[3], path);
+  }
+
   /* This is the _fun_ part: because PHP is utterly idiotic and nonsensical,
   the `call_user_function` macro will __discard__ (yes) its first argument
   (the hashtable), effectively calling functions from `CG(function_table)`.

src/sp_cookie_encryption.h

@@ -11,6 +11,8 @@
 #include "ext/hash/php_hash_sha.h"
 #include "ext/standard/base64.h"
 
+#define SAMESITE_COOKIE_FORMAT "; samesite="
+
 int hook_cookies();
 int decrypt_cookie(zval *pDest, int num_args, va_list args, zend_hash_key *hash_key);
 

src/tests/broken_conf_no_cookie_action.phpt

@@ -0,0 +1,9 @@
+--TEST--
+Bad config, invalid action.
+--SKIPIF--
+<?php if (!extension_loaded("snuffleupagus")) print "skip"; ?>
+--INI--
+sp.configuration_file={PWD}/config/broken_conf_cookie_action.ini
+--FILE--
+--EXPECT--
+[snuffleupagus][0.0.0.0][config][error] You must specify a at least one action to a cookie on line 1.

src/tests/broken_conf_no_cookie_name.phpt

@@ -6,4 +6,4 @@ Borken configuration - encrypted cookie with no name
 sp.configuration_file={PWD}/config/config_encrypted_cookies_noname.ini
 --FILE--
 --EXPECT--
-[snuffleupagus][0.0.0.0][config][error] You must specify a cookie name to encrypt on line 2.
+[snuffleupagus][0.0.0.0][config][error] You must specify a cookie name on line 2.