| Age | Commit message (Collapse) | Author |
|---|
|
Cookies can be arrays (session_id[]=x), and
we called `Z_STRLEN_P(pDest)` without checking if `Z_TYPE_P(pDest) == IS_STRING`,
leading to a type confusion, leaking at least `HashTable->arData`, and the rest
of the code is going to corrupt some stuff, leading to a crash.
While exploitation can't be ruled out, it looks stupidly harder. The array will
be decoded as base64 into another variable, decrypted, and have this value
written back to the array. To obtain a controlled read, an attacker would have
to bruteforce decryption to find an encrypted value that could be properly
decrypted, as we're using authenticated encryption. The next steps are
depending on the heap's layout, which should be pretty deterministic/simple as
cookie decryption happens at RINIT. But since the heap overflow is happening in
the cookies, odds are that they're stored somewhere with other data like
POST/GET/… and thus nothing super-duper juicy. While remote heap exploitation
in PHP have been done in the past, this primitive looks a tad too limited to
yield something powerful enough to pop a shell.
Reported-by: Vozec
|
|
setcookie() is always URL encoded, urlencode is only turned off for setrawcookie().
Turning it off breaks cookies that have a value containing certain characters (namely spaces)
https://github.com/php/php-src/blob/685e99655ae97c667950f7f7d176985958718f56/ext/standard/head.c#L97
|
|
```
========DIFF========
001- OK
001+ Fatal error: Uncaught ValueError: setcookie(): "partitioned" option cannot be used without "secure" option in /builddir/build/BUILD/snuffleupagus-1c7598c432551d0c49c2c57f249ccd5ccabce638/src/tests/samesite_cookies.php:2
002+ Stack trace:
003+ #0 /builddir/build/BUILD/snuffleupagus-1c7598c432551d0c49c2c57f249ccd5ccabce638/src/tests/samesite_cookies.php(2): setcookie('super_cookie', 'super_value')
004+ #1 {main}
005+ thrown in /builddir/build/BUILD/snuffleupagus-1c7598c432551d0c49c2c57f249ccd5ccabce638/src/tests/samesite_cookies.php on line 2
========DONE========
FAIL Cookie samesite [tests/samesite_cookies.phpt]
```
Even though the warning might be spurious, let's fix this properly, by
initialising `partitioned` to false, and by setting it only if `secure` is set
as well.
|
|
|
|
Do not dereference the hash key for cookie encryption if it's NULL:
Program terminated with signal SIGSEGV, Segmentation fault.
#0 zend_string_equal_content (s1=0x79bdb92170f0, s2=0x0) at /usr/include/php/20240924/Zend/zend_string.h:386
No locals.
#1 zend_string_equals (s1=0x79bdb92170f0, s2=0x0) at /usr/include/php/20240924/Zend/zend_string.h:391
No locals.
#2 sp_match_value (value=0x0, to_match=0x79bdb92170f0, rx=0x0) at ./src/sp_utils.c:273
No locals.
#3 0x00007989377b0709 in sp_lookup_cookie_config (key=0x0) at ./src/sp_cookie_encryption.c:8
config = 0x79bdb92158d0
it = 0x79ae80dabd00
it = <optimized out>
config = <optimized out>
#4 decrypt_cookie (pDest=0x79893b6787c0, num_args=<optimized out>, args=<optimized out>, hash_key=0x7ffe657c3880) at ./src/sp_cookie_encryption.c:19
cookie = <optimized out>
#5 0x000061875aac52df in zend_hash_apply_with_arguments ()
No symbol table info available.
#6 0x00007989377ae74b in zm_activate_snuffleupagus (type=<optimized out>, module_number=<optimized out>) at ./src/snuffleupagus.c:228
config_wrapper = 0x7989377c3490 <snuffleupagus_globals+144>
#7 0x000061875aa21710 in zend_activate_modules ()
No symbol table info available.
#8 0x000061875a9a7f18 in php_request_startup ()
No symbol table info available.
|
|
Adjusts casts to void dropping const qualifiers. This helps to avoid
mistakes, e.g. modifying string literals.
Also use size_t for length, similar to the upstream php interfaces.
|
|
* for easier memory manegement, the entire sp_config struct was merged into snuffleupagus_globals and allocated on stack where possible
* SNUFFLEUPAGUS_G() can be written as SPG(), which is faster to type and easier to read
* execution_depth is re-initialized to 0 for each request
* function calls with inline string and length parameters consistently use ZEND_STRL instead of sizeof()-1
* execution is actually hooked if recursion protection is enabled
* some line breaks were removed to make the code more readable
|
|
|
|
|
|
PHP 7.3+ added a new prototype for the cookie
setting mechanism, breaking our ghetto samesite-injection,
this commit takes care of it.
|
|
|
|
* `setcookie` doesn't always return `true` anymore
* clang-format
* Cookies with invalid decryption are dropped, but the request isn't anymore
* faulty unserialize are now dumpable
|
|
|
|
This commit vastly simplifies the code of cookies-fiddling mechanisms.
|
|
This commit does a lot of things:
- Use hashtables instead of lists to store the rules
- Rules that can be applied at launch time won't be tried at runtime
- Improve feedback when writing nonsensical rules
- Make intensive use of `zend_string` instead of `char*`
|
|
* relax test to pass with 7.3
* skip test with 7.3 as samesite is broken + add TODO
|
|
Implement session encryption.
|
|
Refactor the encryption process to extract encrypt/decrypt functions
|
|
|
|
This should close #129
|
|
|
|
|
|
- `clang-format --style="{BasedOnStyle: google, SortIncludes: false}" -i snuffleu*.c sp_*.c sp_*.h`
- Update the documentation accordingly
|
|
It's now possible to encrypt cookies matching a specific regexp.
This should close #106
|
|
This should close #102
This commit can be useful for two use-cases:
1. When deploying Snuffleupagus on big CMS like Magento, and not knowing
what cookies are modified via javascript.
2. When deploying Snuffleupagus on big websites: you don't want to disconnect
every single user at once.
When simulation is enabled, if the decryption fails, a log message is
now issued, and the cookie value taken as it (since odds are that it's
non-encrypted).
|
|
Thanks to this huge commit from @xXx-caillou-xXx, we can now write amazingly flexible rules.
|
|
We can simply use the return value of the original `setcookie` :>
|
|
We forgot to set a return value to the setcookie function, thus always returning false. Since very few frameworks/developers are checking the return value, it went unnoticed until we played with Magento, who effectively checks the return value.
|
|
Apparently, PHP doesn't like when you're trying to save some memory when you're playing with strings.
|
|
- There is no need to generate the key if the cookie has no value
- There is no need to generate the key if the cookie length is invalid
- Use yoda condition
|
|
|
|
|
|
Previously, when a cookie was set with the `httpOnly` flag, it was automatically encrypted, due to a logic flaw. This is now fixed and tested.
|
|
|
|
This is done by using the "samesite" cookie attribute.
|
|
|
|
|
|
|
|
* Fix a cookie encryption issue found by @cfreal
- Use the base64-decoded payload length to allocate memory to decrypt
it, instead of allocating the length of the undecoded one. This has
no security impact, since the base64-encoded string is at least as large
as the decoded one. Since we're using AEAD, there is no way to leak
memory, since this would make the decryption fail.
|
|
|
|
|
