Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Halt font registration when URL fails validation #2995

Merged
merged 1 commit into from Sep 15, 2022

Conversation

bsweeney
Copy link
Member

fixes #2994

@bsweeney bsweeney added this to the 2.0.1 milestone Aug 25, 2022
@bsweeney bsweeney merged commit 66431c5 into master Sep 15, 2022
@bsweeney bsweeney deleted the font-file-validation branch September 15, 2022 13:22
@brandonkelly
Copy link

Any chance we can get a v1 update that includes the same fix? Currently it’s not possible to use Dompdf v1 in conjunction with roave/security-advisories due to this vulnerability.

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - craftcms/commerce 3.x-dev requires dompdf/dompdf ^1.0.2 -> satisfiable by dompdf/dompdf[v1.0.2, ..., v1.2.2].
    - roave/security-advisories dev-latest conflicts with dompdf/dompdf v1.2.2.
    - roave/security-advisories dev-latest conflicts with dompdf/dompdf v1.1.1.
    - Root composer.json requires roave/security-advisories dev-latest -> satisfiable by roave/security-advisories[dev-latest].
    - Root composer.json requires craftcms/commerce @dev -> satisfiable by craftcms/commerce[3.x-dev].

In this case Dompdf is being pulled in from craftcms/commerce v3. We have updated the requirement to use Dompdf v2 in Commerce 4, however we cannot update v3 without introducing potential breaking changes.

@brandonkelly
Copy link

Alternatively if the vulnerability only affects Dompdf >= 2.0, we can get GHSA-6x28-7h8c-chx4 updated in the GitHub Advisory Database.

@bsweeney
Copy link
Member Author

bsweeney commented Oct 4, 2022

Earlier versions were vulnerable to the issue because there was even less validation. The logic related to resource validation significantly changed with the 2.0.0 release, which should have also addressed this bug except for the oversight in not returning on validation error. Because of the nature of the changes I'm not sure about back-porting the fix, I'll need to look into it a bit more.

Is it possible to suggest a Composer alias or override to work around the issue?

@brandonkelly
Copy link

Not sure people who want roave/security-advisories included in require-dev are going to like the idea of pretending their dependencies aren’t vulnerable 🙃

But yeah if it’s not a quick backport, no worries. Just another reason to encourage people to keep things updated.

@bsweeney
Copy link
Member Author

bsweeney commented Oct 4, 2022

Not sure people who want roave/security-advisories included in require-dev are going to like the idea of pretending their dependencies aren’t vulnerable

Yeah, no, I get that. I wasn't thinking of pretending they aren't vulnerable. I was thinking of saying something along the lines of 1.2.3 => 2.0.1.

At any rate, I'll let you know if it looks like we can do a 1.x update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URI validation failure does not halt font registration in Dompdf 2.0.0
2 participants