curl-impersonate-ff: fix build with cmake 4#455108
curl-impersonate-ff: fix build with cmake 4#455108samestep wants to merge 1 commit intoNixOS:masterfrom
Conversation
nixpkgs-ci
bot
added
10.rebuild-linux: 1-10
|
nixpkgs-ci
bot
added
the
12.approvals: 1
| $(brotli_static_libs): brotli-$(BROTLI_VERSION).tar.gz | ||
| tar xf brotli-$(BROTLI_VERSION).tar.gz | ||
| + patch -d brotli-$(BROTLI_VERSION) -p1 < $(BROTLI_CMAKE_PATCH) |
There was a problem hiding this comment.
Uh oh - curl-impersonate-ff uses ancient brotli which is at least vulnerable to CVE-2020-8927.
Considering curl-impersonate is working with internet traffic, this is bad. We should make an attempt to unvendor brotli instead if at all possible.
(github lists a bunch of .NET stuff for that CVE, but https://www.cve.org/CVERecord?id=CVE-2020-8927 says brotli prior to 1.0.8 is affected)
There was a problem hiding this comment.
@LordGrimmauld that seems reasonable to me, but it's orthogonal to this PR, right? That was already an issue long before the CMake 4 change.
There was a problem hiding this comment.
well, kind of. Unvendoring brotli and using our nixpkgs brotli would also fix the cmake issues.
And as it stands, no unsuspecting user will currently install curl-impersonate-ff, because it does not build. If we merge the bodge fix here, then we'd be endandering users again.
So yes, technically the cmake fix can happen before the unvendoring, but i am not a super big fan of that approach...
There was a problem hiding this comment.
That makes sense.
I don't have a preference about whether this PR is merged; I don't use this package (just opened this to help out with the CMake migration) and unfortunately I currently have neither the motivation nor the bandwidth to work on the better fix.
There was a problem hiding this comment.
After bringing this up in the security discussions matrix room, people also noticed curl-impersonate-ff uses old versions of boringssl (not ancient, but old) and does not reliably stay on top of curl updates and more. So yeah, security of this seems like a mess, i won't merge this. I appreciate your attempt at fixing the build, but looking at the security of this, a drop is the more appropriate direction to take this.
There was a problem hiding this comment.
I think this is fine actually, BROTLI_VERSION is 1.0.9 which is the release that fixed CVE-2020-8927. Not arguing with the broader point of -ff being out of date though.
|
@LordGrimmauld I've opened #457684 which drops this package instead of patching it. |
5 participants
Fixes #450649.
cc @deliciouslytyped
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.