update simplesamlphp to 2.3.5 in MOODLE_404_STABLE branch

[hamzakhalem]

please update the simple saml it has vulnerability in process

[danmarsden]

thanks - we're hoping to have team members starting on a new build when the new version of simplesamlphp is released next week. If you have development capabilities feel free to help by submitting a PR with the changes. (note our version of Simplesamlphp has extra patches on top).

more info here:
https://github.com/catalyst/moodle-auth_saml2/blob/MOODLE_39_STABLE/.extlib/UPGRADE.md

[hamzakhalem]

thanks - we're hoping to have team members starting on a new build when the new version of simplesamlphp is released next week. If you have development capabilities feel free to help by submitting a PR with the changes. (note our version of Simplesamlphp has extra patches on top).

more info here: https://github.com/catalyst/moodle-auth_saml2/blob/MOODLE_39_STABLE/.extlib/UPGRADE.md

sure i will try too :D

[danmarsden]

we do have an internal testing build with a newer version of simplesamlphp - but not one that includes the incoming security patch so we haven't published it here - we'll wait for upstream to fix the main build and then look to update the version here including the security patch.

[dasistwas]

Hi, I wanted to ask if you have any updates on that issue. There is already the fix published:

https://simplesamlphp.org/security/202412-01

[danmarsden]

@dasistwas the new MOODLE_404_STABLE branch includes Simplesamlphp 2.3.3 as per:
https://github.com/catalyst/moodle-auth_saml2?tab=readme-ov-file#branches

We do need to update that to 2.3.5 but hopefully that won't be too difficult if someone wants to help out.

Unfortunately the older branch (MOODLE_39_STABLE) which is used for 4.1 needs to support PHP 7.4 so the 2.0.X branch is the newest branch we can use as simplesaml 2.1 requires PHP 8.0

It's not clear to me if the vulnerability reported affects the 2.0 version of Simplesamlphp

We do note that it appears to relate to loading a xml document, and we think that anyone who can do that within auth_saml2 is already an admin on the site but I haven't fully traced the details so open to feedback from anyone that does a deeper investigation too.

[danmarsden]

just had a further look at this today - the vulnerability was related to the xml-common library which doesn't exist in the 2.0 version we have in the MOODLE_39_STABLE branch (but it is in the new 404_STABLE branch.)

new 404_STABLE branch shows it here:
https://github.com/catalyst/moodle-auth_saml2/tree/MOODLE_404_STABLE/.extlib/simplesamlphp/vendor/simplesamlphp

and you can see it's not in the older branch:
https://github.com/catalyst/moodle-auth_saml2/tree/MOODLE_39_STABLE/.extlib/simplesamlphp/vendor/simplesamlphp

[dasistwas]

Hi @danmarsden, thank you for the detailed research. The options to remove from XML parsing mentioned there:

https://simplesamlphp.org/security/202412-01

can be found there:

https://github.com/simplesamlphp/saml2/blob/717c0adc4877ebd58428637e5626345e59fa0109/src/SAML2/DOMDocumentFactory.php#L41

These options are also in the MOODLE_39_STABLE branch:

https://github.com/simplesamlphp/saml2/blob/717c0adc4877ebd58428637e5626345e59fa0109/src/SAML2/DOMDocumentFactory.php#L41

The recommendation from https://simplesamlphp.org/security/202412-01 is:

Remove the LIBXML_DTDLOAD | LIBXML_DTDATTR options

So not sure if the code is used. But I created a pull request with the fix recommended in https://simplesamlphp.org/security/202412-01

#854

[hamzakhalem]

@dasistwas good job bro

[juancs]

Hi :-) This link added more information to me:

https://nvd.nist.gov/vuln/detail/CVE-2024-52806

The patch can be seen here:

simplesamlphp/saml2@5fd4ce4

@danmarsden, for what I'm seeing from the patch, DOMDocumentFactory class (the one affected by the thing) exists too in MOODLE_39_STABLE branch and, for what I see in the patch, It's affected too.

I'm using MOODLE_39_STABLE branch (and I guess that all Moodle installations as well) so maybe we can mitigate the probleam too.

I've patched the plugin that way and eveything is still working. Maybe I've missed something.

@dasistwas , maybe you can take a look at this information too.

If a patch is required I can provider one.

[dasistwas]

Hi @juancs ,

thank you for referencing the fix. I updated my pull request and included the fix (without unit test and the PHP8.4 modification - which is not needed for the Moodle version in 39 branch because Moodle does not support PHP8.4 yet).

You can have a look at that commit:

3bae74e

I tested this on one production site:

https://intern.diehauswirtschaft.at

It seems to work fine.

[dasistwas]

But maybe it is better to update directly from https://github.com/simplesamlphp/saml2 ? Not sure how the external lib is integrated/updated in this plugin.

[danmarsden]

I don't have any objections to adding the suggested patch to the older branch although from what I can see the actual vulnerability is inside xml-common - the DOMDocumentFactory class passes the data to xml-common (in the newer simplesamlphp) - and the older branch doesn't use that library for parsing the xml. (it uses normal PHP functions from what I could see.)