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

why is it possible to execute arbitrary system commands via python code through the electrum (Qt) console #3678

Closed
ghost opened this issue Jan 9, 2018 · 24 comments
Labels
security 🔐 technical issue that affects security of funds

Comments

@ghost
Copy link

ghost commented Jan 9, 2018

wondering why it is currently possible to execute arbitrary system commands via python code through the electrum console, and why there is no whitelisting/blacklisting of allowed methods/functions as this seems like a major security issue

inserting a payload such as below in the console, then checking /tmp for the file creation shows the file was successfully created

lol=import('os');lol.system('/bin/sh -c "touch /tmp/test"');print(lol.path.isfile('/tmp/test'));
True

@attritionorg
Copy link

Dupe to #3374 and fixed by 3.0.5 per changelog?

@ghost
Copy link
Author

ghost commented Jan 9, 2018

ehh @attritionorg this is not a dupe of that issue by any means, and has nothing to do with rpc requests whatsoever.

its moreso because there is no proper sandboxing within the electrum codebase whatsoever, thus allowing direct execution of system commands by utilizing python code such as the example i provided above from within the electrum console

@AbdussamadA
Copy link
Contributor

so you can execute python code in the python console? why do you think this is a security risk? anyone who has access to that can just as well run python directly.

@ghost
Copy link
Author

ghost commented Jan 9, 2018

@AbdussamadA its a security issue because there is no sandboxing/whitelisting of allowed or disallowed methods to be called, and you most definitely should not be able to execute system level commands from a wallet which is designed to store your funds in

@AbdussamadA
Copy link
Contributor

I recommend this be closed because it's just another junk issue.

@ghost
Copy link
Author

ghost commented Jan 9, 2018

@AbdussamadA has no understanding of this and no clue as to what hes talking about, and this is definitely not another junk issue. perhaps you should go read one of the many papers on practices for execution of untrusted code before you make such remarks buddy.

@ghost
Copy link

ghost commented Jan 9, 2018

i can see many relevant reasons as to why this is a security issue, but it seems a bit obvious @AbdussamadA hasnt the first clue in regards to his aforementioned claims.

@SmileyChris
Copy link
Contributor

For someone read on the practices for execution of untrusted code, one would hope you also understand responsible disclosure procedures and wouldn't just make a public ticket about what you perceive to be a security vulnerability 😫

Is there is any possibility for remote execution?

@ysangkok
Copy link
Contributor

ysangkok commented Jan 9, 2018

I don't think there is any indication of remote execution being possible.

Many applications have a scripting console, for example FreeCAD. For example, if you have online banking, there is a JavaScript console in your browser, which is then having full rights to do whatever you can do with your online banking solution.

Sandboxing is a solution for running untrusted code. The code you write in the Python console is trusted. Therefore sandboxing is not applicable.

So I would argue that the Python console is accepting arbitrary Python as it should.

We will consider adding a warning to prevent users from being tricked into pasting malicious scripts. This purpose is to prevent Self-XSS.

@The-Compiler
Copy link

You can't realistically blacklist Python code to make it "secure", and you also can't realistically sandbox it except on an OS level.

That being said, I think it's fine that the Python console can, in fact, run Python 😉

@ghost
Copy link
Author

ghost commented Jan 11, 2018

consider the scenario that a user/co-worker leaves electrum open and walks away from their desk, while they do have their wallet encrypted with a password,
having the password does not matter,
because the console can be abused by another party to run a snippet of python which forks a new thread, runs an infinite loop, and implements a hook that hooks the methods responsible for decrypting the wallet, and sending transactions, then proceeds to send a small percentage of the current balance to address of choice as well as exfiltrating an unencrypted copy of the wallet to any endpoint of the users choosing.

i have a proof of concept that achieves this successfully however i will not share it here due to security concerns.

the only way this can be circumvented is by sandboxing or whitelisting allowed functionality which most likely will not happen, or removal of the console all together, which will still leave users who have not updated open to being attacked in this manner.

also @The-Compiler its been long known that pysandbox is broken and can be bypassed, however there are other working methods of sandboxing python. no need to link to a 5+ year old article on a random blog, or a 3+ year old article on lwn stating something that was already known for quite some time before that article was even written

@bauerj
Copy link
Collaborator

bauerj commented Jan 11, 2018

Well, consider this scenario then:

A user/co-worker leaves electrum open and walks away from their desk, while they do have their wallet encrypted with a password. Since the operating system is unlocked, an attacker could simply install a keylogger and copy the wallet.

It doesn't matter for this kind of attack if you can execute arbitrary code in Electrum or on the OS level.

@ghost
Copy link
Author

ghost commented Jan 11, 2018

actually @bauerj it does matter because in the scenario youve outlined, installing a keylogger would leave a detectable artifact behind on the users system as well as require more privileges than a regular user would most likely have where as in the scenario i outlined it does not leave an easily detected artifact behind on the system, and the code responsible for carrying this out is gone as soon as electrum is closed.

judging by your response and the lack of understanding of what ive stated above, i assume you are not very security savvy

id also like to point out that your assumption of them copying the wallet would not be feasable as the wallet is stored on the disk in an encrypted state, and only decrypted when read into memory by electrum, thus reiterating the point ive raised above in regards to you not being security savvy

id also like to point out that adding a warning as stated above by @ysangkok is only an obscure means of preventing this as the malicious party injecting this payload would just ignore the warning, and also, there is nothing in regards to this that would allow something such as self xss to occur in this context as electrum is not built from javascript/html, and the console does not execute such payloads.

@SomberNight
Copy link
Member

SomberNight commented Jan 11, 2018

@n0bd

why there is no whitelisting/blacklisting of allowed methods/functions

Can you detail what you think should be white/blacklisted so that

  1. the console remains functional
  2. it's not a security risk in the way you outline

Excuse my doubt, but I very much think that these two points are contradictory.

I also think that if an attacker has a chance to physically access your machine, unmonitored, it is already game over at that point. The fact that you also mention having the OS running, unlocked, with Electrum open, just makes it... even more game over?

But if people think this is a real concern, I propose either

  1. disabling the console by default with a manual opt-in to enable it for a given session only; potentially with a warning that the console has been enabled; and with no option to disable the console for that session (as otherwise rogue threads would have to be hunted down and killed which is out of scope)
  2. disabling the console by default with an option to enable/disable it persistently; but disabling would need an app restart

EDIT: haha, I've just realised that the console itself can be used to circumvent my proposals. There is no reasonable way to keep it functional and alleviate OP's concerns.

@ecdsa
Copy link
Member

ecdsa commented Jan 11, 2018

actually the console is disabled by default, isnt it?

@bauerj
Copy link
Collaborator

bauerj commented Jan 11, 2018

It is.

@SomberNight
Copy link
Member

I've added an EDIT, but I think I'll also make it a separate comment:

haha, I've just realised that the console itself can be used to circumvent my proposals. There is no reasonable way to keep it functional and alleviate OP's concerns.

@ecdsa
Copy link
Member

ecdsa commented Jan 11, 2018

I just took the time to read this thread.. sorry @n0bd but I think you do not know what you are talking about.

There is no indication that remote execution is possible. If the attack vector is local, the presence of the python console does not add any extra threat compared to the presence of the program in memory. The only risk I see is with social engineering attacks, and that is why the console is disabled by default.

@ecdsa ecdsa closed this as completed Jan 11, 2018
@ghost
Copy link
Author

ghost commented Jan 11, 2018

@ecdsa i guess you looked over the part where i stated that this is local, not remote, however this is still a viable security issue, and since youve chosen to close this bug report, i guess ill go with the latter option and submit my poc and details to mitre for a cve request.

i wasnt going to take this path, but by choosing to close this issue, youve made a statement to the public that you arent really concerned about the security of your users so mitre it shall be

@attritionorg
Copy link

MITRE will assign an ID regardless of the merit of the report, FYI. If you include the URL to this ticket in your request for an ID, MITRE will either assign and mark it DISPUTED or opt not to assign pending more details.

@ghost
Copy link
Author

ghost commented Jan 11, 2018

@attritionorg im already aware of that. i was attempting to give them the opportunity to fix this without sending a report to mitre so they didnt take another hit and get 2 cve's in a weeks time, but since they chose to close the issue and write it off then it looks like thats the path im going to end up taking

@SomberNight
Copy link
Member

@n0bd

Can you detail what you think should be white/blacklisted so that

Do you have any suggestions as to what to change? Concrete suggestions.
Do you want the console stripped out of the app? Because that seems to be the only realistic approach so far.

@ghost
Copy link
Author

ghost commented Jan 27, 2018

just adding a comment to let the community know that mitre assigned CVE-2018-6353 to this issue

@bauerj
Copy link
Collaborator

bauerj commented Jan 27, 2018

Thanks for letting us know. The social engineering part should be addressed with #3700.

@SomberNight SomberNight added the security 🔐 technical issue that affects security of funds label Feb 5, 2020
@SomberNight SomberNight changed the title why is it possible to execute arbitrary system commands via python code through the electrum console why is it possible to execute arbitrary system commands via python code through the electrum (Qt) console Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security 🔐 technical issue that affects security of funds
Projects
None yet
Development

No branches or pull requests

9 participants