Fix unpacking of the sessionId
authorTim Düsterhus <duesterhus@woltlab.com>
Wed, 18 Aug 2021 07:44:07 +0000 (09:44 +0200)
committerTim Düsterhus <duesterhus@woltlab.com>
Wed, 18 Aug 2021 07:55:20 +0000 (09:55 +0200)
commit3f2cf3b4d439d79d404312f4bf3362d980101f4a
treed5f240be3ff801068a62fc432bac8ee019690d07
parent32282ca3c984f4a4ef3ca82fd8708944b8f97b43
Fix unpacking of the sessionId

As documented by PHP's reference documentation:

> The "a" code now retains trailing NULL bytes.
> The "A" code now strips all trailing ASCII whitespace (spaces, tabs,
> newlines, carriage returns, and NULL bytes).

Previously, with the 'A' code, sessionIds ending in ASCII whitespace would be
incorrectly unpacked, missing their trailing bytes. This ultimately resulted in
the session not being found and the user being logged out.

Five of the 256 possible characters exhibited this bug, making this fail in
roughly 2% of the cases.

However this likely was not noticable by the typical user. Once they have a
non-affected sessionId, this Id is not going to change. What the user might've
noticed is a login not working, despite showing a success message, because they
sessionId change after a successful login handed out an affected sessionId. But
then the user would likely try again, succeeding this time and writing off the
incident as a fluke.

Test script to reproduce the issue:

    <?php

    for ($i = 0; $i <= 255; $i++) {
        $string = "foo".chr($i);

        $packed = \pack(
            'CA4',
            1,
            $string
        );
        $unpacked1 = \unpack('Cversion/A4string', $packed);
        $unpacked2 = \unpack('Cversion/a4string', $packed);

        if ($unpacked1['string'] !== $string) {
            echo "$i: unpacked1\n";
        }
        if ($unpacked2['string'] !== $string) {
            echo "$i: unpacked2\n";
        }
    }
wcfsetup/install/files/lib/system/session/SessionHandler.class.php