buttercup / buttercup-desktop

:key: Cross-Platform Passwords & Secrets Vault
https://buttercup.pw
GNU General Public License v3.0
4.33k stars 331 forks source link

Buttercup fails to open due to ASLR incompatibilities #502

Open ghost opened 6 years ago

ghost commented 6 years ago

W10 Pro _x64 1709 b16299.125 BC 1.0.1

With W Defender ASLR exploit set to On by default BC would fail to start

bc fail start

Overriding the ASLR setting in W Defender gets BC up and running

aslr Please make BC compliant with ASLR

perry-mitchell commented 6 years ago

We currently use Electron to build our apps - We don't have, to my knowledge, direct control over the built artifacts. The best we can do at this stage is to sign the executable and installer. I believe this should be resolved once that's been done. All subject to time and costs of course.

ghost commented 6 years ago

code signature and ASLR are somewhat different.

ghost commented 6 years ago

Meantime reached BC 1.8 and W b16288.547 and the issue is present as ever

perry-mitchell commented 6 years ago

Happy to receive help on this. We don’t currently have the time nor experience to check this out. Happy to work with someone on it of course..

ghost commented 6 years ago

This open now for 4 months, any outlook when there might be time?

The other statement about experience is quite frank but honest at least but a bit shocking all the same, for a password handling app not implementing Address Space Layout Randomization.

BC is the sole app that has this issue. Anyway, perhaps time to move on then as opposed to be moaning. Coding is not wihtin my portfolio and thus not able to lend a hand, I am afraid.

perry-mitchell commented 6 years ago

@n8v8R I think that's a cheap shot.. You're the only person so far to have mentioned this issue, and it's still obscure enough that we haven't ranked it as a higher prio.

The other statement about experience is quite frank but honest at least but a bit shocking all the same, for a password handling app not implementing Address Space Layout Randomization.

The funny thing is many people phrase their disappointment like this.. that they're surprised we don't implement feature A or security requirement B. Don't get me wrong, we want all of this.. And we'd be only too happy for our software to be as compliant, secure and stable as possible. But in terms of most important to least important things we need to add, I still don't see ASLR as being anywhere near the start of that list. There are numerous other security precautions we should take before even considering it - such as the auto-locking of all archives on every device, or the encryption stability on mobile..

Hopefully you can understand that our time is mega limited and we need to take all advice with a grain of salt. If something is getting a bit more attention or initial research into the problem reveals that it is critical, we'll normally take it sooner. But again this all boils down to the fact that we simply cannot build everything on Buttercup.. It's open source so people can help us do that.

BC is the sole app that has this issue

I highly doubt this - for instance...

perry-mitchell commented 6 years ago

@n8v8R I'll reach out on the issue to see what others think on the matter, but at this early stage I'd guess this issue is with Electron, the foundation. If this is the case then we simply can't do anything about the issue for now.

perry-mitchell commented 6 years ago

I'll keep this open as we want a resolution.

ghost commented 6 years ago

@perry-mitchell

not sure how you conclude me straying from the subject or not being helpful.

What is not helpful is that this not getting fixed for months and then the priority being lowered even. Next thing you know there is the curious Reddit post as opposed of getting in touch with the Electron devel instead, considering it being the supposed root cause in your reckoning. How does that make sense?

ASLR is a commonly denominated security feature for all major OS and nothing particular to WIN. So much could have been learned from a bit of reasearch. And BC being a password handling (java script) app a user of BC could expect that its devel are indeed familiar with such.

That BC and just few other apps facing this issue is no indication at all that Electron is the root cause but rather the contrary - being a matter of implementation, e.g. apparently WhatsApp Desktop does not exhibit such issue and has likely a larger user base than BC.

If you look at https://github.com/electron/electron/blob/master/electron.gyp there is something mentioned of ASLR.

You keep on citing being short on resources, which I am not missing, to handle the project. That rather raises more questions than it answers. I do not think it helps the cause of the project/app if it cannot properly maintained as that is not about faancy feature but a security basic. Sure, that would not raise an eyebrow if it were for some app not involving any security aspects at all but for a password handling app it is rather dubious argument (being short on resources).

Under the circumstance I do cocur with your point of view that any further discussion on the subject is rather futile.

perry-mitchell commented 6 years ago

@n8v8R This is getting rediculous. I’ve already admitted to not being familiar with ASLR, and have explained numerous times that right now, this has not been a priority for us. I’m not going to go through why again.

There have been other security related items that have taken precedence, and on top of that, Windows is currently not the primary system we target or test on. We do understand that the majority of our users will be on Windows, but obviously very few have ASLR enabled. Windows will take focus when we have more devs on board.

Your assuming and condescending attitude has definitely put us off, as well. I would suggest that you leave this be now, unless you have something to offer in terms of fixing the issue or linking an implementation or resolution. We appreciate the initial report and your support for the feature. If you continue to debate our legitimacy or priorities I’ll unfortunately have to block your account on our repositories.

perry-mitchell commented 6 years ago

Confirmed that overriding the only ASLR setting that appears to be off by default (windows 10), the issue occurs immediately:

image

Lodged an issue first on Electron builder: electron-userland/electron-builder#3188

perry-mitchell commented 6 years ago

Ok, I think I cracked it. Seems that the build environment plays a part in the problem here. Our binaries are built on Mac for Mac, Linux and Windows. Mac and Linux are easy obviously.. the build systems are of course radically different but there doesn't seem to be any incompatibilities. Building Windows binaries on a Mac startled me when I first heard of it, but it seems like NSIS does a good job and we hadn't noticed any problems with Buttercup.

I did however build a new installer (Buttercup 1.9.0) on Windows and installed that. It worked under ASLR immediately!

So whatever the issue is, it's something to do with the NSIS builder on Linux. I'll file an issue there, but for now it seems like we need to build windows installers on Windows only.

Updated the existing issue: https://github.com/electron-userland/electron-builder/issues/3188

perry-mitchell commented 6 years ago

Quick update: The issue seems to be with one or some of the signing tools used by Electron builder. We're working with that team to debug the problem.

pomeroythomas commented 5 years ago

@perry-mitchell

Just FYI I ran into this exact issue on v1.16.2 for Windows x64 and disabling ASLR did fix it.

image

perry-mitchell commented 5 years ago

Happy to receive a PR for this - it’s a bit outside our area of expertise (Windows).