Skip to content

Switch to NSIS 3.03 to avoid DLL hijacking #13643

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.

Sign up for GitHub

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

Jump to bottom
Closed
h4x3rotab wants to merge 2 commits into from
Closed

Switch to NSIS 3.03 to avoid DLL hijacking #13643

h4x3rotab wants to merge 2 commits into from

Conversation

@h4x3rotab
Copy link

@h4x3rotab h4x3rotab commented Jul 12, 2018 •
edited
Loading

Early version of NSIS searches its DLL from the same directory of the executable. If a hacker can place some DLL files in the same directory of the bitcoin installer, the installer will load and run it with admin permission.

Gitian is still in trusty. It shipped with NSIS 2.46, which is vulnerable to this issue. So in this fix, we instead build the latest NSIS by Gitian.

Thanks to @wilsonmeier from Bitcoin Gold team for the fix. Borrowed some code from TOR project.

Details: https://trac.torproject.org/projects/tor/ticket/17895

@fanquake fanquake requested a review from theuni July 12, 2018 14:09
@laanwj
Copy link
Member

laanwj commented Jul 12, 2018

Thanks,
The idea was to switch to Ubuntu 18.04 for building to avoid having to do this, as that ships with a newer NSIS package, but if that turns out not to be feasible before the next release this will be helpful!

@wilsonmeier @h4x3rotab
Switch to NSIS 3.03 to avoid DLL hijacking
921c372 
Early version of NSIS searches its DLL from the same directory of
the executable. If a hacker can place some DLL files in the same
directory of the bitcoin installer, the installer will load and
run it with admin permission.

Gitian is still in trusty. It shipped with NSIS 2.46, which is
vulnerable to this issue. So in this fix, we instead build the
latest NSIS by Gitian.

Thanks to @wilson from Bitcoin Gold team for the fix. Borrowed some
code from TOR project.

Details: https://trac.torproject.org/projects/tor/ticket/17895
@maflcko
Copy link
Member

maflcko commented Jul 12, 2018

if that turns out not to be feasible before the next release this will be helpful!

Currently our Windows gitian cross builds are broken, so we'd have to switch to bionic. (Or revert the qt depends bump)

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 12, 2018 •
edited
Loading

Note to reviewers: This pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot mentioned this pull request Jul 12, 2018
Merged
@ken2812221
Copy link
Contributor

ken2812221 commented Jul 12, 2018

This has been fixed in nsis 2.50, and nsis in ubuntu bionic is version 2.51.

@h4x3rotab
Copy link
Author

h4x3rotab commented Jul 12, 2018 •
edited
Loading

This has been fixed in nsis 2.50, and nsis in ubuntu bionic is version 2.51.

Yeah, bionic could be even better.

@theuni
Copy link
Member

theuni commented Jul 12, 2018

Holding out on reviewing this until after #13171 is merged or closed.

@ghost
Copy link

ghost commented Jul 14, 2018

Bitcoin Gold developer contributing for Bitcoin Core, nice 👍

@maflcko
Copy link
Member

maflcko commented Jul 16, 2018

Closing per #13643 (comment)

@maflcko maflcko closed this Jul 16, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@theuni theuni Awaiting requested review from theuni

Assignees

No one assigned

Labels

Build system Windows

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@h4x3rotab @laanwj @maflcko @DrahtBot @ken2812221 @theuni @fanquake @wilsonmeier