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

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

Closed
vkhramtsov opened this issue Feb 1, 2022 · 27 comments
Closed

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

vkhramtsov opened this issue Feb 1, 2022 · 27 comments
Assignees

Comments

@vkhramtsov
Copy link

vkhramtsov commented Feb 1, 2022

Hello.

We are still using symfony 4.4.* on our project, but commit 94a98d3 (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.

@vkhramtsov vkhramtsov changed the title Last commit conflicts with symfony/framework-bundle: 4.4.* Latest commit conflicts with symfony/framework-bundle: 4.4.* Feb 1, 2022
@hvt
Copy link

hvt commented Feb 1, 2022

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

@hvt
Copy link

hvt commented Feb 1, 2022

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

@vkhramtsov
Copy link
Author

@fabpot could you please comment here?

@hvt
Copy link

hvt commented Feb 1, 2022

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

@Ocramius
Copy link
Member

Ocramius commented Feb 1, 2022

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

Still, feel free to keep discussing this

@Ocramius Ocramius closed this as completed Feb 1, 2022
@fabpot
Copy link

fabpot commented Feb 1, 2022

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
Copy link
Author

vkhramtsov commented Feb 1, 2022

@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
Copy link

fabpot commented Feb 1, 2022

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

@vkhramtsov
Copy link
Author

vkhramtsov commented Feb 2, 2022

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
Copy link
Author

Sent message to github support about this issue.

@fabpot
Copy link

fabpot commented Feb 2, 2022

We have reported the issue to Github already.

@jurgenhaas
Copy link

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
Copy link
Author

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

@jurgenhaas
Copy link

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

@vkhramtsov
Copy link
Author

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

@jurgenhaas
Copy link

Well, the linked issue symfony/symfony#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
Copy link
Author

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

@fabpot
Copy link

fabpot commented Feb 3, 2022

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
Copy link
Member

Ocramius commented Feb 3, 2022

BTW, similar woes at laminas/laminas-form#162 (comment)

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
Copy link

jurgenhaas commented Feb 3, 2022

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
Copy link
Member

Ocramius commented Feb 3, 2022

@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
Copy link

jderusse commented Feb 3, 2022

FYI it has been fixed on github side: GHSA-vvmr-8829-6whx

@Ocramius
Copy link
Member

Ocramius commented Feb 3, 2022

Can anyone verify if the version range now makes sense? 0c86f70

@vkhramtsov
Copy link
Author

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

@jurgenhaas
Copy link

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

Fahl-Design added a commit to Fahl-Design/SecurityAdvisoriesBuilder that referenced this issue Feb 3, 2022
- use interfaces whenever possible
- Add a rule provider with factory to add rules
- Added unit tests to verify Advisory rules a

ref Roave/SecurityAdvisories#90

Signed-off-by: Benjamin Fahl <git@fahl-design.de>
Fahl-Design added a commit to Fahl-Design/SecurityAdvisoriesBuilder that referenced this issue Feb 3, 2022
- add unit tests to verify rules are applied as expected

ref Roave/SecurityAdvisories#90

Signed-off-by: Benjamin Fahl <git@fahl-design.de>
Fahl-Design added a commit to Fahl-Design/SecurityAdvisoriesBuilder that referenced this issue Feb 3, 2022
@vkhramtsov
Copy link
Author

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

@Ocramius
Copy link
Member

Ocramius commented Feb 4, 2022

Meanwhile, WIP patch/discussion to allow for workarounds, should this occur again: Roave/SecurityAdvisoriesBuilder#528

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

No branches or pull requests

6 participants