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

Archive fetched by fetchFromGitHub is unpacked regardless of hash check #63564

Closed
Chiiruno opened this issue Jun 20, 2019 · 7 comments
Closed

Comments

@Chiiruno
Copy link
Contributor

Issue description

I was experimenting with fixing the ue4 package, and noticed that the archive, when fetched from fetchFromGitHub is unpacked regardless of the hash check.
Couldn't this be a possible security risk if say, something crazy like a code execution exploit was MITM'd in it's place or something else? (Although, the gzip and tar code is pretty solid)
We should be hash checking the sha256 of the archive before doing anything with it.

@FRidh
Copy link
Member

FRidh commented Jun 20, 2019

fetchFromGitHub uses fetchzip when possible. Zip implementations typically don't produce archives deterministically, thus requiring us taking the hash over the contents of the archive.

@FRidh
Copy link
Member

FRidh commented Jun 20, 2019

Actually, I think we could also use fetchurl because GitHub can produce other types of tarballs as well. Unfortunately, such a change would invalidate all existing hashes.

@Chiiruno
Copy link
Contributor Author

Perhaps a staging PR for changing fetchFromGitHub, and a related issue for correcting the new hashes for all affected packages should be made?

@FRidh
Copy link
Member

FRidh commented Jun 20, 2019

There are also uses of fetchFromGitHub outside Nixpkgs, which should be considered. In principle there is no reason for fetchFromGitHub, one can use fetchurl or fetchgit just fine.

Closing because there isn't really anything to do.

@FRidh FRidh closed this as completed Jun 20, 2019
@Chiiruno
Copy link
Contributor Author

Yes, but a lot of packages use fetchFromGitHub, so I still think this needs to be addressed, unless there is a plan to remove fetchFromGitHub entirely.
Other methods that use fetchzip should be considered for this too where applicable.
I think you should re-open this, it's still a glaring vulnerability.

@FRidh FRidh reopened this Jun 20, 2019
@Chiiruno
Copy link
Contributor Author

As far as "uses of fetchFromGitHub outside Nixpkgs" goes, perhaps a notice where applicable to let users know of such a change, and to replace the hash might suffice.

@Chiiruno
Copy link
Contributor Author

Looks like you're probably right about there being nothing to do, at least in the scope of nixpkgs.
Closing, if anyone has any ideas, feel free to re-open.

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

No branches or pull requests

2 participants