Vulnerability in phenx/php-svg-lib

[Whip]

Dompdf is dependent on phenx/php-svg-lib v0.3 to 1.0. There is a security vulnerability in this repo in versions below 0.5.1.

+-------------------+----------------------------------------------------------------------------------+
| Package           | phenx/php-svg-lib                                                                |
| CVE               | CVE-2023-50251                                                                   |
| Title             | Denial of service caused by infinite recursion when parsing SVG document         |
| URL               | https://github.com/advisories/GHSA-ff5x-7qg5-vwf2                                |
| Affected versions | <0.5.1                                                                           |
| Reported at       | 2023-12-13T13:32:21+00:00                                                        |
+-------------------+----------------------------------------------------------------------------------+

Could you modify the dependency to use a newer version of this library? Meanwhile, would it be safe to run composer update phenx/php-svg-lib or will things break?

[bsweeney]

I'm trying to coordinate the releases. SvgLib will be moving to 1.0, which will be the new minimum for Dompdf 3.0.

In the meantime you can update via composer without breaking anything. The changes have been primarily around resource validation. Dompdf 2.0.4 also includes some additional relevant validations.

[heddn]

Is there a usage of dompdf/dompdf w/ phenx/php-svg-lib that is secure? The SA at GHSA-97m3-52wr-xvv2 says dompdf/dompdf itself is also vulnerable and w/ composer audit feature, my CI builds are falling over themselves. I'm confused what is a fixed combination of versions. 😕

[ms-abm]

Is there a usage of dompdf/dompdf w/ phenx/php-svg-lib that is secure? The SA at GHSA-97m3-52wr-xvv2 says dompdf/dompdf itself is also vulnerable and w/ composer audit feature, my CI builds are falling over themselves. I'm confused what is a fixed combination of versions. 😕

same here

[bsweeney]

Dompdf with that most recent release of SvgLib is secure. The separate SA in not because Dompdf itself is vulnerable, but because it's vulnerable in the context of using SvgLib. There's a greater rating in the Dompdf SA since the attack surface is generally greater when SvgLib is used with Dompdf. So, the Dompdf SA is primarily to raise awareness of the issue with Dompdf users.

I don't know if there's a way to indicate in the SA that Dompdf is only vulnerable based on the SvgLib dependency (e.g., Dompdf in combination with SvgLib <=0.5.1 is affected), which is why it shows * in the affected/patched fields. It's also why I didn't request a CVE since the issue is not specifically with Dompdf.

[bsweeney]

I removed Dompdf from the list of affected packages, which I expect would allow a pass with composer audit? If anyone has guidance on a better way to configure the SA let me know.

[stilliard]

Confirmed running composer update phenx/php-svg-lib upgrades just that lib without issue.

What do you think about updating the composer.json to require 0.5.2 as the minimum version and releasing a new minor version?
"phenx/php-svg-lib": ">=0.5.2 <1.0.0"
Can test locally & submit a PR if you'd like?

[ohader]

Some side notes on Phar meta-data deserialization (I spent a bunch of time on that topic in 2018):

• Phar meta-data deserialization was fixed in PHP 8.x and thus only affects older PHP versions
• for PHP 7.x and PHP 5.x we created a corresponding package that intercepts potential malicious phar:// invocations
  → https://packagist.org/packages/typo3/phar-stream-wrapper

[bsweeney]

What do you think about updating the composer.json to require 0.5.2 as the minimum version and releasing a new minor version?

Version 3.0.0 is pretty much ready to go and will require dompdf/php-svg-lib 1.0. I'm just finalizing changes across our libraries. Though if everyone would prefer a minor release that requires SvgLib 0.5.2 I'm happy to oblige.

[stilliard]

Thanks & understood.

With version 3.0.0, is it a simple upgrade? It might be worth having that temp release in between for the SvgLib 0.5.2 security fix for people not quite ready to upgrade if there's breaking changes? & as a quick fix for any one else facing this in between?

I've fixed it for my repos though with the composer update command so I guess it depends if other people would find it useful?

[SCIF]

@bsweeney

What do you think about updating the composer.json to require 0.5.2 as the minimum version and releasing a new minor version?

Version 3.0.0 is pretty much ready to go and will require dompdf/php-svg-lib 1.0. I'm just finalizing changes across our libraries. Though if everyone would prefer a minor release that requires SvgLib 0.5.2 I'm happy to oblige.

There is the advisory marking dompdf as vulnerable (even though it has wider constraint) and roave/security-advisories doesn't allow to use dompdf based on the advisory. It seems like there are two ways to overcome this:

1. minor release tightening the constraint
2. contacting Github advisory team in order to take out dompdf from the advisory list. I assume this will take ages and minor release is much less disruptive way

[bsweeney]

I'll need to contact Github regardless since, apparently, there is no patched version listed.

[SCIF]

I'll need to contact Github regardless since, apparently, there is no patched version listed.

Sure, but automated advisory checkers could break building pipelines

[bsweeney]

I'm not saying I won't release 2.0.5 just that I may need to contact Github to make sure it flows through to that advisory. Particularly if, as noted, roave is blocking installs because of it.

[gcumbaya]

What do you think about updating the composer.json to require 0.5.2 as the minimum version and releasing a new minor version?

Version 3.0.0 is pretty much ready to go and will require dompdf/php-svg-lib 1.0. I'm just finalizing changes across our libraries. Though if everyone would prefer a minor release that requires SvgLib 0.5.2 I'm happy to oblige.

I would really prefer the additional minor release as it currently breaks pipelines for security reasons. Upgrading to a new major version with new features i would like to be able to schedule that instead almost being enforced due a vulnerable dependency.

🙏

[glennmcewan]

I don't see why dompdf/dompdf has been marked as affected by this Advisory?

dompdf allows tags for the affected package between 0.3.3 and 1.0.0 -- our composer.lock file is now up-to-date with 0.5.2 of phenx/php-svg-lib, yet composer audit still fails.

In my experience it seems atypical to create a new tag simply to enforce a new minimum of a package's dependencies upon discovering a vulnerability in one of its requirements when it's perfectly valid for consumers of dompdf/dompdf to upgrade the peer dependencies themselves...

[bsweeney]

I don't see why dompdf/dompdf has been marked as affected by this Advisory?

dompdf allows tags for the affected package between 0.3.3 and 1.0.0 -- our composer.lock file is now up-to-date with 0.5.2 of phenx/php-svg-lib, yet composer audit still fails.

In my experience it seems atypical to create a new tag simply to enforce a new minimum of a package's dependencies upon discovering a vulnerability in one of its requirements when it's perfectly valid for consumers of dompdf/dompdf to upgrade the peer dependencies themselves...

You are not wrong. GHSA-97m3-52wr-xvv2 listed Dompdf as an informational. It was intended to alert the community that there's an expanded attack surface when Dompdf is used with any version of SvgLib prior to 0.5.2 (and PHP < 8, but there's no way to indicate something like that). But the system is opaque to me, and I didn't realize listing Dompdf without specific version information would create the situation we're currently in. That's on me, lesson learned.

I'm still working through if a minor release will be required. I submitted an update to the global advisory, and we'll see what happens from there. I do have a minor release ready to go if the attempted resolution does not clear the current pipeline issues.

[LorenzoRogai]

It was merged on advisory-database. The alert should now have gone away for those that upgraded php-svg-lib to 0.5.2

[bsweeney]

Thanks for watching and reporting back. I was waiting to see what would happen with that.

[alexmigf]

@bsweeney do you have an estimated date for the next minor release? Just trying to align it with our planning. Thanks for all the hard work!

[bsweeney]

I was thinking of ... not? Since dependency managers should retrieve the patched release of SvgLib without issue. But if y'all want to see a minor release that bumps the minimum requirement of SvgLib I'm happy to do so.

[glennmcewan]

I agree that a new tag is not necessary for dompdf/dompdf. This can be remedied via composer update phenx/php-svg-lib if you need to be restrictive with which dependencies are updated. A CVE assigned to dompdf/dompdf <= 2.0.4 was a false alarm.

[bsweeney]

FYI if you want to enforce a minimum SvgLib version with your Dompdf installation without specifying it in your composer you can upgrade to 2.0.7.