simplesamlphp / saml2

SimpleSAMLphp low-level SAML2 PHP library
https://www.simplesamlphp.org
GNU Lesser General Public License v2.1
284 stars 134 forks source link

Serialization of SAML2\XML\saml\NameID creates corrupted serialized data #350

Closed CSkjerris closed 8 months ago

CSkjerris commented 8 months ago

When NameID is serialized as part of a SimpleSAML\Session, the part of the serialized string that represents the NameID objects will have "\0*\0" prepended infront of the attributes, which results in the deserialization of the object to turn bad.

Additionally, when using this with e.g. a DB session handler, the insert statement will end once it reads the "\0", so it will lose some data.

Example of bad string:

"O:18:"SimpleSAML\Session":10:{s:9:"sessionId";s:32:"e7e00504b2b33f9a1afee38052b0ba68";s:9:"transient";b:0;s:7:"trackid";s:10:"4aa9159b02";s:16:"rememberMeExpire";N;s:5:"dirty";b:0;s:19:"callback_registered";b:0;s:9:"dataStore";a:2:{s:22:"\SimpleSAML\Auth\State";a:1:{s:43:"_f232a367dd719313128b565c794f16434159549ead";a:3:{s:7:"expires";i:1706199814;s:7:"timeout";i:3600;s:4:"data";s:920:"a:8:{s:16:"saml:logout:Type";s:5:"saml2";s:15:"saml:logout:IdP";s:31:"https://saml.test-nemlog-in.dk/";s:18:"saml:logout:NameID";O:21:"SAML2\XML\saml\NameID":6:{s:16:"*NameQualifier";N;s:18:"*SPNameQualifier";s:29:"https://saml3.lt.netp.stil.dk";s:11:"*nodeName";s:11:"saml:NameID";s:9:"*Format";s:52:"urn:oasis:names:tc:SAML:2.0:nameid-format:persistent";s:15:"*SPProvidedID";N;s:8:"*value";s:83:"https://data.gov.dk/model/core/eid/person/uuid/3ecec2ec-dbb1-4dac-96dc-bccc6e4c2339";}s:24:"saml:logout:SessionIndex";s:59:"4D-E7-98-EA-FD-20-FB-95-DD-25-FE-0A-32-F2-D6-5D-E6-1F-07-B2";s:8:"ReturnTo";s:64:"https://lt.netp.stil.dk/login/logout.php?logout&isLoggedOut=true";s:22:"LogoutCompletedHandler";a:2:{i:0;s:22:"SimpleSAML\Auth\Simple";i:1;s:15:"logoutCompleted";}s:25:"\SimpleSAML\Auth\State.id";s:43:"_f232a367dd719313128b565c794f16434159549ead";s:28:"\SimpleSAML\Auth\State.stage";s:12:"saml:slosent";}";}}s:39:"\SimpleSAML\Auth\Source.LogoutCallbacks";a:1:{s:50:"16:nemlogin-test-lthttps://saml.test-nemlog-in.dk/";a:3:{s:7:"expires";s:17:"sessionEndTimeout";s:7:"timeout";s:17:"sessionEndTimeout";s:4:"data";a:2:{s:8:"callback";a:2:{i:0;s:22:"SimpleSAML\Auth\Source";i:1;s:14:"logoutCallback";}s:5:"state";a:1:{s:36:"\SimpleSAML\Auth\Source.logoutSource";s:16:"nemlogin-test-lt";}}}}}s:12:"associations";a:0:{}s:9:"authToken";s:43:"_2a8a74da2014c5acb126f6d2c7aac95772a276bc78";s:8:"authData";a:0:{}}"

Notice that NameIdQualifier has "*" infront and the "\0"'s wasn't part of the copied out response from my debugger, however, the "\0"' explains why ti this the lenght of "NameQualifier" is 16 compared to 13.

I have a temp fix on my own application, where I added the following code to NameID:

    public function __serialize(): array
    {
        return [
            'NameQualifier' => $this->NameQualifier,
            'SPNameQualifier' => $this->SPNameQualifier,
            'nodeName' => $this->nodeName,
            'Format' => $this->Format,
            'SPProvidedID' => $this->SPProvidedID,
            'value' => $this->value
        ];
    }
    public function __unserialize($serialized): void
    {
        foreach ($serialized as $k => $v) {
            $this->$k = $v;
        }
    }
Nyuwb commented 7 months ago

Hi @tvdijen do you plan to release a v4.6.12 for this fix ? I had to downgrade our dependency because of this issue. Thanks !

tvdijen commented 7 months ago

The issue was resolved in v4.6.11, but you have to clear your session store. Old existing sessions cannot be unserialized using the new unserialize-methods.

Nyuwb commented 7 months ago

It seems like I misunderstood : we will stay in v4.6.11 and I'll clear the session store, it should be better like that. Thanks a lot !

tvdijen commented 7 months ago

No worries! I'll add a note to the changelog for future reference