Roave / SecurityAdvisories

:closed_lock_with_key: Security advisories as a simple composer exclusion list, updated daily
MIT License
2.72k stars 106 forks source link

Latest commit conflicts with symfony/framework-bundle: 4.4.* #90

Closed vkhramtsov closed 2 years ago

vkhramtsov commented 2 years ago

Hello.

We are still using symfony 4.4.* on our project, but commit https://github.com/Roave/SecurityAdvisories/commit/94a98d36257ecb87064ae581a2e04b381119ac57 (latest commit currently) conflicts with all versions symfony/framework-bundle from 4.4 branch (branch 4.4 still maintained https://symfony.com/releases/4.4). I suppose that this is because of https://symfony.com/blog/cve-2022-23601-csrf-token-missing-in-forms, but this is relates to package with verions 5 and 6.

Would you be so kind to clarify, why do you have this in composer.json? Would you be so kind to fix this?

Thank you in advance.

hvt commented 2 years ago

~Looking at the (probable) source of the advisory, there appears to be going something wrong with matching of the specific branches.~

hvt commented 2 years ago

Mmm, now that I look further I think the GitHub Advisory causes this.

vkhramtsov commented 2 years ago

@fabpot could you please comment here?

hvt commented 2 years ago

(I also notified Symfony's security team by email.)

Ocramius commented 2 years ago

Closing here meanwhile: the issue is not with roave/security-advisories itself, but with https://github.com/advisories/GHSA-vvmr-8829-6whx having wrong version ranges.

Still, feel free to keep discussing this

fabpot commented 2 years ago

As replied to the security email, that's a Github issue. I've tried to change the constraints to see if that fixes the issue, but I cannot do much more than that.

vkhramtsov commented 2 years ago

@fabpot could you change affected version like you do for symfony 6? For example >= 5.0.0, < 5.3.15 or =5.3.15

fabpot commented 2 years ago

I have made the change, but apparently, there is a cache.

vkhramtsov commented 2 years ago

Problem still exists. @fabpot, @Ocramius do you have any possibility to push github advisories update or revoke current security advisory and create new one?

vkhramtsov commented 2 years ago

Sent message to github support about this issue.

fabpot commented 2 years ago

We have reported the issue to Github already.

jurgenhaas commented 2 years ago

Sorry to comment on a closed issue, but I haven't found a better place for this yet: our Drupal updates are blocked because of this since Monday and I wonder, is there anything we can do to raise the attention at GitHub's site to actually deal with this?

vkhramtsov commented 2 years ago

@jurgenhaas from my point of view we can only write new emails to github support.

jurgenhaas commented 2 years ago

@vkhramtsov is that support@github.com ? If so, I would just send one too.

vkhramtsov commented 2 years ago

@jurgenhaas I cannot guarantee that it will work, but it could help

jurgenhaas commented 2 years ago

Well, the linked issue https://github.com/symfony/symfony/issues/45271 states, that @fabpot had fixed it, and it only requires a cache rebuild in this repo here, and that issue has also been closed, but it doesn't have to worked yet.

vkhramtsov commented 2 years ago

@jurgenhaas so now we have to wait for guthub security advisory update.

fabpot commented 2 years ago

Unfortunately, there is nothing more we can do. We have created a Github support ticket almost immediately, that is more than 2 days ago now. I've tried to change the constraints on our side to make them more explicit. And I've just tweeted about the issue: https://twitter.com/fabpot/status/1489139646730747904

In any case, the solution in the meantime is to temporarily disable this package and do the update.

Ocramius commented 2 years ago

BTW, similar woes at https://github.com/laminas/laminas-form/issues/162#issuecomment-1028696649

I think the GitHub advisory distribution chain is a bit problematic as it is currently designed in GitHub itself, because these kinds of problems occur all the time, and going through their support seems to be a really inefficient modus operandi.

EDIT: ref https://twitter.com/Ocramius/status/1489146984363708418

jurgenhaas commented 2 years ago

Thanks for all the feedback. Another resolution might be to replace Roave/SecurityAdvisories with a forked version where we remove problematic entries. Problem being of course that we have to keep that up-to-date.

Ocramius commented 2 years ago

@jurgenhaas rather than doing that, I'd say that (assuming github advisories keep staying this unreliable long-term), https://github.com/Roave/SecurityAdvisoriesBuilder could be adjusted to have logic to exclude some advisories manually, via hardcoding.

From my PoV, it's an acceptable trade-off, to allow the community to continue benefiting from the exclusion range, even if something is awfully wrong in github.

See https://github.com/Roave/SecurityAdvisoriesBuilder/blob/24711c25c30198a7fa1e28d4aefb0e57e7a181a7/src/Roave/SecurityAdvisories/AdvisorySources/GetAdvisories.php#L31

The Generator produced there can be filtered, and select advisories can be kicked out.

EDIT: please consider sending a patch (I'm currently swamped)

jderusse commented 2 years ago

FYI it has been fixed on github side: https://github.com/advisories/GHSA-vvmr-8829-6whx

Ocramius commented 2 years ago

Can anyone verify if the version range now makes sense? https://github.com/Roave/SecurityAdvisories/commit/0c86f70adfb4d976d59e222d2c36590919158039

vkhramtsov commented 2 years ago

It is works at least in my setup. I'm going to check additionally tomorrow,.

jurgenhaas commented 2 years ago

Great, it works here too - so glad it's fixed.

vkhramtsov commented 2 years ago

Additional checks passed. Everything works as expected. Thanks everyone.

Ocramius commented 2 years ago

Meanwhile, WIP patch/discussion to allow for workarounds, should this occur again: https://github.com/Roave/SecurityAdvisoriesBuilder/pull/528