Fix the decoding of filter settings cookie

[fmeum]

(refs #29)

Previously, the use of json_decode without a second paramter meant that an stdClass was returned, which does not allow access to properties via the index operator. Instead, we now use json_decode(..., true) to return an associative array.

In order to prevent any type shenanigans, we also check whether the returned value is an array and if not, replace it with an empty one.

[fmeum]

Please also verify that this change does not break GOsa for users with a cookie generated by serialize.

[master-caster]

serilized values won't work with json_decode so you need to clean your cookies.

$test = ["a" => ["d", "e"], "b", "c"];
php > var_dump(serialize($test));
string(68) "a:3:{s:1:"a";a:2:{i:0;s:1:"d";i:1;s:1:"e";}i:0;s:1:"b";i:1;s:1:"c";}"
php > var_dump(json_decode(serialize($test)));
NULL
php > var_dump(json_decode(serialize($test), true));
NULL

[fmeum]

Is it really required to clear cookies? The intention of the check after the decode is to ensure the array would be initialized to an empty array. Then the site would add the defaults and overwrite the cookie with a JSON-encoded one, effectively resetting the settings but not requiring users to explicitly clear the cookies. I will run some more local tests.

[master-caster]

we could add some code to check the return value of json_decode

if it is NULL we unserilize the value. But this is code getting useless after first run. So cleaning your cookies is uncomfortable for the user but seems to be the "best" solution.

But of course i'm open to other proposals :-)

[fmeum]

if it is NULL we unserilize the value.

This would reintroduce the original vulnerability, since anyone could still pass arbitrary input to an unserialize call. Short of implementing our own unserialize replacement, I don't see a way to preserve the original settings. Are the filter settings cumbersome to redo for the user?

[master-caster]

i never did any filter settings on purpose but i know you need to recreate for all of your plugins since they are stored on a plugin based structure for each user.

[sunweaver]

Hi,

On Mo 19 Aug 2019 11:01:39 CEST, Benjamin wrote:

i never did any filter settings on purpose but i know you need to recreate for all of your plugins since they
are stored on a plugin based structure for each user.

how about ignored stored filter settings, if the serialize/unserialize
code got used the last time a user connected? Simply set them to
defaults and populate a new cookie via the json implementation?

Mike --

DAS-NETZWERKTEAM c\o Technik- und Ökologiezentrum Eckernförde Mike Gabriel, Marienthaler str. 17, 24340 Eckernförde mobile: +49 (1520) 1976 148 landline: +49 (4351) 850 8940

GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22 0782 9AF4 6B30 2577 1B31 mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

[fmeum]

@sunweaver If I understand you correctly, this is basically what the current patch does (or at least is supposed to do): If the cookie does not contain valid JSON (which serialize will never produce), the settings array will be empty, which causes the same behavior as with no cookie. The existing code then sets a new cookie with the defaults.