CVE-2024-52596 in simplesamlphp/simplesamlphp

Updated our composer.json packages with the following changes:

 - Upgrading simplesamlphp/saml2 (v4.6.14 => v4.16.14)
 - Upgrading simplesamlphp/simplesamlphp (v2.1.3 => v2.3.5)
 - Upgrading simplesamlphp/simplesamlphp-assets-base (v2.1.19 => v2.3.2)

Everything seems to work fine as long as also the simplesamlphp is also the same latest version.

Doesn't hurt to update it, we still allow 1.x though, that is EOL and will never be updated, so doesn't really matter. Sites are responsible to update third party dependencies in case of security issues

This needs to be updated and a new releaseneeds to be pubished. This is NOT the responsibility of the sites. The maintainers need to publish a new release.

Regarding #4: Looking at https://www.drupal.org/psa-2024-06-26 → and especially at https://www.drupal.org/psa-2022-06-20 → I do think it's the responsibility of individual sites to update their dependencies.

Can you explain more why you think it's the responsibility of the maintainers to bump the version?

I agree with berdir that this might be out of scope for the module to totally handle because bumping the composer.json version might not be enough but you need to do work outside of Drupal to update simpleSAMLphp. But not all developers have for example Dependabot monitoring in place so I would think it would make sense for the module to safe guard a bit and only allow installing a safe version of the module? Especially since the CVE score seems to be quite high.

It's not a security issue in the actual module so I don't think this is worth a SA-CONTRIB notice or anything like that. This is why I also opened a public issue for this one. I have seen previously similar issues where a third party library has a vulnerability and there have been similar fixes that for example composer.json is locked to version v2.3.5 so that it only supports safe versions of the module.

What is the reason to keep supporting old EOL versions of simpleSAMLphp or dev-simplesamlphp-2.1 version of it?

This was in fact raised as a security issue and closed due to the PSA references in #5. Not even core does that anymore.

composer audit does work for this and reports this issue.

> What is the reason to keep supporting old EOL versions of simpleSAMLphp or dev-simplesamlphp-2.1 version of it?

Because we technically still support Drupal 9. simpleSAMLphp is tied to the symfony version, you can only use 1.x on D9. Of course D9 is EOL too, so if you're stuck there, you're on unsupported and insecure versions anyway.

The dev version is just a leftover of when there wasn't a stable release yet, that can be removed.

I'm happy to merge an MR and also release this, but it will just be a regular release and not a security update and the official position per #5 is very clear on this.

I created the MR now. I added the minimum version for 2.3.5 and removed the old dev option.

Hope I did it as intended.

Can you elaborate where was this raised as a security issue? In Drupal security issue tracker that is not public?

This module isn't compatible with Drupal 11. We need to configure CI to only run on previous major and disable current.

> Can you elaborate where was this raised as a security issue? In Drupal security issue tracker that is not public?

Yes.

Is there a related issue for the pipeline update I could mention here?

And thanks for the elaboration.

No existing issue yet, feel free to do it here in this MR. See https://www.drupal.org/drupalorg/blog/gitlab-ci-templates-will-make-drup... → , just a few variables that need to be set.

I adjusted the gitlab-ci.yml and the composer.json and added the following variables:

OPT_IN_TEST_CURRENT: '0'
OPT_IN_TEST_PREVIOUS_MAJOR: '1'

Composer install and phpunit now pass with Drupal 10. It still tries to install Composer dependencies for Drupal 11 which seem to fail. I'll dig a bit more from the documentation if I can disable also the composer install for the next major version and only run composer install for the previous major.

The documentation was a bit confusing because according to https://www.drupal.org/drupalorg/blog/gitlab-ci-templates-will-make-drup... → the shift has not yet happened from Drupal 10 to Drupal 11 but 📌 Update templates so 11.0 is the default/current branch RTBC seems closed and fixed. At least the setting seems to now skip Drupal 11 phpunit correctly and and only test Drupal 10.

I'll mark this as Needs review for now.

I think it's because the validate jobs depend on the composer jobs, we'd need to change them to run on previous major as well.

Can figure out later, but will ask in Slack #gitlab if there's a documentation page.

See https://drupal.slack.com/archives/CGKLP028K/p1733405684090189:

The simplest way is to pin you gitlab template to the latest version prior to when we made the switch to D11. This is as per 
@fjgarlin
 commment above. It is a one-line change, to alter 'ref'
include:
  - project: $_GITLAB_TEMPLATES_REPO
    ref: 1.5.x-latest

Changed now and the pipelines pass with warnings. The warnings seem to be related to files outside of this issues scope.

I disabled the next major tests for now although they were only warnings so if you prefer, we can only pin the gitlab template to that ref and still keep OPT_IN_TEST_NEXT_MAJOR enabled.

Changed the .gitlab-ci.yml to change DRUPAL_CORE variable instead of changing ref, then it updates to the latest D10 version.

Also fixed the version constraint. You almost never want to use >= because that would also allow any larger version, including 3.0.0 and up. The correct thing to use here is ^, which allows patch and minor updates, while ~would only allow patch updates.

I also thought about the version constraint but seems I didn't think far enough. The original thought I had would was to put something like there:
>=2.3.5 <3.0

That would force it to over 2.3.5 but not over 3.0.0 but then there might of course be situations like 2.4.0 etc. but if we still allow minor versions, then that would also work.

But this works for me also. Thanks for the extra effort!

> >=2.3.5 <3.0

Yes, that is exactly the same as ^2.3.5, just shorter.

Merged.

Automatically closed - issue fixed for 2 weeks with no activity.