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

weechat relays give remote code execution rights #928

Closed
anarcat opened this issue Mar 21, 2017 · 20 comments
Closed

weechat relays give remote code execution rights #928

anarcat opened this issue Mar 21, 2017 · 20 comments
Assignees
Labels
feature New feature request
Milestone

Comments

@anarcat
Copy link

anarcat commented Mar 21, 2017

this IRC conversation made me gave up on relays and, incidentally, weechat itself:

2016-08-12 16:46:43     anarcat ouch, the weechat protocol gives RCE
2016-08-12 16:47:47     sim642  RCE?
2016-08-12 16:47:59     Yaniel  remote code execution
2016-08-12 16:48:15     sim642  You mean like /exec?
2016-08-12 16:48:45     anarcat yes
2016-08-12 16:48:58     anarcat didn't expect that
2016-08-12 16:49:18     sim642  weechat relay gives everything weechat gives
2016-08-12 16:49:26     sim642  You could unload the exec plugin
2016-08-12 16:49:37     sim642  but that prevents nothing, any script can still execute things
2016-08-12 16:50:40     Yaniel  and nothing prevents you from loading the exec plugin again
2016-08-12 16:50:50     sim642  Deleting it would :P
2016-08-12 16:51:03     Yaniel  well...
2016-08-12 16:51:11     Zarthus you kinda need a shell account to have access to weechat anyways
2016-08-12 16:51:14     @nils_2 or using another client :p
2016-08-12 16:52:29     anarcat well, the irc relays don't give you that level of control

in other words, if i hook up my Android phone to a Weechat instance using the "weechat" relay protocol, i give that phone execution rights to the machine running the weechat instance, as the user running weechat. the impact of this is huge: if you lose that phone, that device can then be used to infiltrate your network. the communication channel can also be attacked to try and abuse that new attack surface.

i do not believe users expect this from the weechat relays. 2 new users that were happy to start using Weechat did not know about this feature and were really surprised to hear about /exec (which they didn't know about at all). a friend proposed the following workaround:

$ sudo mv /usr/lib/weechat/plugins/exec.so /usr/lib/weechat/plugins/disabled-exec.so
weechat> /plugin unload exec

but one should note this will fail when the package is updated. in Debian, this can be worked around with apt-mark hold weechat-plugins, but then upgrades are completely disabled, which is a security issue in itself.

note that, on top of deleting the exec plugin, you will probably also want to disable any scripts plugin, that is lua, python, perl, ruby, tcl, guile, javascript... and probably xfer as well. at that point, you will have disabled most of the extra functionality in weechat and it suddently looks much less interesting.

i therefore strongly petition the weechat team to consider setting the relay protocol to allow setting a allow_list of /commands allowed to certain relays, that defaults to "none" or maybe just /me, /join, /part, and so on...

@anarcat anarcat changed the title weechat relays should not have the right to run any command weechat relays give remote code execution rights Mar 21, 2017
@weechatter weechatter added the feature New feature request label Mar 21, 2017
@cfggns
Copy link

cfggns commented Mar 21, 2017

Since the exec.so file is part of the weechat-plugins package in debian, the last step of the workaround should actually be apt-mark hold weechat-plugins. All of the other potentially risky plugins that anarcat mentions are also in the weechat-plugins package, along with the relay and script plugins are also (which makes sense). https://packages.debian.org/sid/amd64/weechat-plugins/filelist

Also, I don't think that /exec sudo mv /usr/lib/weechat/plugins/exec.so /usr/lib/weechat/plugins/disabled-exec.so would actually run. I didn't have much luck trying to run anything but the simplest commands through /exec (still very likely enough for RCE though!), and there would be no interactivity for sudo auth anyways. The command works in a normal shell though (not through weechat).

As I understand it, all of the different language plugins are all for running scripts, so they depend on the script plugin to get the code to run, and then load. So a manual disabling of the script plugin once all desired scripts have been installed and loaded effectively disables all of these script plugins as potential ways of pulling off a RCE. And, without having tested it at all, the file transfer capacity of the xfer plugin seems like it could maybe bring in some code, and disabling it doesn't seem like it would break anything, so that would probably be a good idea too. Also the trigger plugin says "execute one or more commands" in its description, so that one too.

So once desired scripts have been setup, this should hopefully harden weechat against the kinds of attacks via a less-trusted relay (a phone!) that are being raised in this issue, while still leaving it pretty fully featured:

sudo mv /usr/lib/weechat/plugins/exec.so /usr/lib/weechat/plugins/disabled-exec.so
sudo mv /usr/lib/weechat/plugins/script.so /usr/lib/weechat/plugins/disabled-script.so
sudo mv /usr/lib/weechat/plugins/xfer.so /usr/lib/weechat/plugins/disabled-xfer.so
sudo mv /usr/lib/weechat/plugins/trigger.so /usr/lib/weechat/plugins/disabled-trigger.so

And then "/plugin unload" for all of these within weechat. Of course, changing the scripts that are running in weechat will then require a manual enabling and then loading of the scripts plugin.

Since these are all installed via the weechat-plugins package, the apt-mark hold weechat-plugins mentioned above is going to prevent the package manager from undoing the changes. I agree this is in itself a security problem (especially for any important updates to relay.so itself, which is included in weechat-plugins), and it would be much better all around to be able to have an "untrusted relays" feature to restrict what commands relays can run on the server!

@anarcat
Copy link
Author

anarcat commented Mar 21, 2017

I do not know if trigger would execute stuff once exec is disabled.

I have modified the original post to cover the mistakes you have outlined, thanks!

@flashcode
Copy link
Member

Hi,

The weechat protocol allows client to display a GUI and let the user enter commands exactly like if it was locally in WeeChat itself.
It's a feature, not a bug and not a security issue.

Restricting allowed commands in relay client is not easy, because /exec or scripts can be indirectly called by other commands (like triggers, /script, ...).

I think it's better to protect correctly the relay plugin, by using a non-standard port, restricting IP allowed, and of course setting a strong and secure password (which is NOT saved on the client side for maximum security).

If needed, we can think about even more security on the relay plugin, which is for me a better solution than trying to restrict what the client can do.

Running commands via /exec and the relay client is similar than a SSH access on your box with a SSH key (and optional password). If someone get your device, he can connect as your user and do anything with the shell.

As a conclusion, I would remind that relay plugin is purely optional and never enabled by default in WeeChat. So when it is enabled, the user is responsible to secure it correctly (as I mentioned above) to prevent any problem.

Ideas are welcome to protect even more connections from relay clients.

@anarcat
Copy link
Author

anarcat commented Mar 21, 2017

you assume the client is trusted. what i am saying is this is a false assumption: people are not aware that the client can run all those commands, and are pretty liberal about what machines they authorize to connect to the weechat relay.

regarding the protection you are proposing users do:

  • change the default port: this only means an attacker will need to do a portscan to perform a bruteforce attack. this is also unnecessary if a device is lost
  • restricting allowed IPs: will break mobile access (e.g. from Android)
  • strong and secure password: useless against passive attackers that can sniff the traffic (e.g. any public wifi hotspot)

even worse: the client, by default connects in the clear, over plaintext TCP, to the relay port. this allows arbitrary commands to be injected through through TCP session hijacking. arguably, any sane person wouldn't do that and would instead setup a VPN or SSH bridge below that, but instructions like the Weechat Android guide don't even mention that.

I am really not sure people are aware that the relay service provides such capabilities. it seems to me it would be really important to clarify, in the documentation, that the exec commands and the like execute in the context of the server, and not the client. this is a huge surprise for me coming from the Irssi world, where /exec is executed on the client, and not the server.

and SSH is different: there are ways to craft SSH credentials (through authorized_keys restrictions) that do not give remote code execution and only give port forwarding access (for example).

besides, i am deliberately not giving SSH access on the box when i setup a Weechat relay: the point is exactly to provide limited functionality, namely to chat with people and not run arbitrary commands.

you can call this a feature all you like, I certainly did not think of Weechat relays as remote shells. if they are, that should really be clarified, especially considering some people use public web interfaces that they do not even control themselves to connect to their own Weechat relays... basically, anyone using glowing-bear.org is giving them a shell on their local relay.

if that's considered acceptable security practices in the weechat project, that makes me seriously question the practices elsewhere in the project... are there any hidden shells like this in other parts of the client that we should know about?

@anarcat
Copy link
Author

anarcat commented Mar 21, 2017

to clarify, the kind of attack i am thinking of, once someone has control over a weechat client (through TCP session hijacking, stealing your phone or script injection on glowing bear), is some nasty shit like:

/exec -sh sh | nc -l -p 19992

that would give anyone a remote shell on the server running weechat on port 19992.

nice feature. one has to wonder who would want to make something like that even possible...

@flashcode
Copy link
Member

First, security is not an option in WeeChat, it's very important.
There is no backdoor, no remote code execution, no hidden feature to give rights to external users, nothing hidden in WeeChat. All the code is public, just audit it and if you find something wrong, just ping me.

As I said, relay gives the client exactly same rights as WeeChat itself, it was designed like that.
The protocol is NOT designed to restrict the client user. So it's not a good idea to let clients connect if you don't want them to execute any command via /exec or similar commands. Relay is just a "remote display" of WeeChat itself, not a way to run only some commands from a remote host.

And it assumes clients are trusted, that's why it's protected by IP and/or password, and of course SSL support.
If you don't trust clients, you should not use relay (weechat protocol) at all.

If you want restricted access, there are 3 solutions:

  1. use irc relay protocol, with that weechat is just an IRC gateway, commands like /exec can not be run from clients
  2. add mechanisms in weechat relay to restrict command that are run, but like I said it's hard to do that by design
  3. add another relay protocol, which would be between irc and weechat, with different goals.

Another thing, there is nothing in plain text by default: there is no IRC server by default in WeeChat (since version 1.2), and no relay. When you define the relay, you can of course use SSL (and I recommend that). This is explained in /help relay.
If some clients use plain text by default, it's not a WeeChat issue, please contact the author of the relay client. But anyway if you have setup SSL on WeeChat side, you'll not be able to connect as plain text from any client.

I'm OK to mention in docs clearly that with weechat relay protocol you can run any command.

@flashcode flashcode added question General question and removed feature New feature request labels Mar 21, 2017
@anarcat
Copy link
Author

anarcat commented Mar 21, 2017

i'm just going to give up here because it seems we have drastically different views on how this should work. i understand your point that weechat relay system is not designed to restrict commands, and i'm not arguing about that. what I am arguing is that the design is fundamentally flawed and urgent action is required to avoid serious security issues.

but just to be clear:

  1. Weechat relay is not just a remote "display". it's a fully featured client, which includes remote code execution, among other things.
  2. I do not consider this to be an undisclosed security vulnerability, because in the conversation I had over IRC, developers seemed aware of the issue, which is why i didn't go through the usual disclosure process. but I do believe it is a serious security issue that people should be aware of, and should probably have a CVE assigned
  3. i never trust machines to run code on other machines, except in extreme circumstances (e.g. configuration management like puppet or other sorts of automation designed for that purpose. in that sense, there are no trusted machines. so because "If you don't trust clients, you should not use relay (weechat protocol) at all", I would argue that no one should use the Weechat relay protocol unless they clearly intend to give Remote Code Execution rights to other clients
  4. the relay is in plaintext by default. it's not enabled by default, you are right, but the default is without SSL, which is a separate option that is not enabled by default in the relay. the password is, similarly, unset by default.

My recommendations would be:

  1. this issues should be clearly documented in the relay documentation
  2. password and SSL should be mandatory
  3. there should be a whitelist or blacklist of allowed commands for weechat clients
  4. if that needs to be implemented in a different protocol, the weechat relay protocol should be deprecated and removed in future releases
  5. all third party weechat plugins prominently advertised in the download page should be contacted and be made aware of this issue.

Unfortunately, I do not have the time nor the energy to go through this whole process myself. This issue made me avoid weechat completely and i'm happy to be using another client right now. I just opened this issue to document it for my fellow IRC users that recently switched to Weechat without being aware of this design issue.

@flashcode
Copy link
Member

flashcode commented Mar 21, 2017

So my answers:

  1. WeeChat relay is just a remote "display". The relay lets you type commands exactly like if you were typing them in WeeChat. It doesn't allow anything more. You can not run more commands or execute more things in the relay than in WeeChat itself.
  2. The relay plugin is completely optional in WeeChat, not enabled by default, and has options to secure it. So when you enable it, you know what you are doing. As I said, I'm OK to add more info in docs about what you can exactly do in the relay client (the same things as in WeeChat itself).
    For me there's no CVE on that, it's a feature (that must be well documented), not a security issue.
  3. As I said, relay client can not do anything more than WeeChat itself.
  4. The relay is NOT plain text by default. There is no relay* configured by default in WeeChat. So when you define one, you can choose if you use SSL or not. Of course, SSL is recommended.
    Moreover, since WeeChat 1.6, an empty password is not allowed by default any more (a new option was added to control that). So if you don't setup a password, no client can connect.

About your recommendations:

  1. I'll add info in docs about what relay clients can do.
  2. Password is mandatory (unless you explicitely set the option to allow empty passwords). For me there's no need to force SSL: some users are using relay through a SSH tunnel or locally (on the same host), in these cases SSL is not needed on relay side. But I can improve docs to say it's highly recommended if you don't use a tunnel and if you are accessing your WeeChat from internet.
  3. I can add an option to white/blacklist commands, the default value has to be discussed (not allowing some commands can disturb existing relay clients).
  4. Implementing another protocol would not deprecate weechat protocol since this protocol is a feature and is completely optional. If needed I can add an option to NOT compile weechat relay protocol if you want to be sure noone is using it.
  5. I'm sure developers of relay client know what clients can do: they know that the weechat relay protocol is just a way to execute commands in WeeChat like if you were typing them in WeeChat.

@flashcode flashcode added doc Project documentation feature New feature request and removed question General question labels Mar 21, 2017
@carnager
Copy link

carnager commented May 7, 2017

@ashkitten You didn't really read what @flashcode said, did you? If you want something like irssi-proxy use /relay add ssl.irc, which acts as a pure irc proxy with all networks in one connection.
weechat protocol is neither enabled per default nor is it the only available protocol of /relay.

@dato
Copy link

dato commented Oct 20, 2017

(Just passing by.)

In Debian, this can be worked around with apt-mark hold weechat-plugins, but then upgrades are completely disabled, which is a security issue in itself.

You can instruct dpkg to mark the renaming of the file as permanent with:

$ sudo dpkg-divert --rename --local /usr/lib/weechat/plugins/exec.so

This will rename the file to /usr/lib/weechat/plugins/exec.so.distrib, and keep it there across updates of the package too (thus getting security fixes, etc. etc.).

@sudoforge
Copy link

I think the main issue here is that people confuse Weechat's Relay mode with a standard bouncer like ZNC or psyBNC, when instead it provides an interface more akin to SSH or VNC.

@shibumi
Copy link

shibumi commented Feb 25, 2019

well, weechat relay has some advantages over IRC relay mode.. I use the weechat relay as well and I would prefer having weechat relay over IRC relay for all my relays, but this code execution is ridiculous.

small addition: Weechat supports since 2.4 TOTP. This way you can secure your weechat relay with a unique password and TOTP as second factor. The client needs to support it.

@flashcode flashcode removed the doc Project documentation label Feb 26, 2019
@flashcode flashcode self-assigned this Feb 26, 2019
@flashcode flashcode added this to the 2.5 milestone Feb 26, 2019
@flashcode
Copy link
Member

flashcode commented Feb 26, 2019

To mitigate the problem, I'll add a new option relay.weechat.commands which is a list of allowed/excluded commands.

The default value could be something like *,!exec,!upgrade,!quit to allow any command except /exec and /quit.
To be even more secure, the value could be set to a list of allowed commands, like: 'away,buffer,join,me,query,…`.

Note: commands indirectly called via aliases or scripts will also be affected (ie allowed or excluded, according to the value of the new option).

@flashcode flashcode added the in progress Someone is working on this issue label Feb 26, 2019
@flashcode flashcode removed the in progress Someone is working on this issue label Feb 28, 2019
@mhoran
Copy link
Contributor

mhoran commented Mar 8, 2019

This is great, thanks @flashcode! Just pointing out that one would likely want to add !set to the default list (if blacklisting) as well, since an attacker could otherwise use /set to override this setting.

@flashcode
Copy link
Member

@mhoran: you're right. For maximum security, I'll disable these commands as well: /fset, /set, /unset, /plugin, /script (and scripting plugins commands like /python) and /secure.

flashcode added a commit that referenced this issue Mar 9, 2019
Commands were already forbidden (option relay.weechat.commands):

- /exec
- /upgrade
- /quit

These extra commands are now forbidden by default:

- /fset
- /set
- /unset
- /plugin
- /script
- /python
- /perl
- /ruby
- /lua
- /tcl
- /guile
- /javascript
- /php
- /secure
@oakkitten
Copy link

i propose a whitelist instead of a blacklist for the default setting. if someone is trying to execute malicious code, they'll likely spend considerable time investigating their options, and blacklisting gives them plenty of commands to work with

fwiw running /wait 1s /exec uptime seems to be bypassing the blacklist and it took me like 5 minutes to discover this

@flashcode
Copy link
Member

@oakkitten: for /wait, you're right it can be used to execute any command, so I'll add it to the list (more important changes could be done on /wait to be forbidden if the command itself to execute later is forbbidden). I'll also add /repeat to the list.

Using a whitelist is harder, I just counted about 131 commands that should be allowed, while 26 should be forbidden (if we count aliases as well). So the list of allowed commands would be extremely long.
Anyway this is just the default value, you can use a whitelist if you want to be very strict and allow only some commands.

@oakkitten
Copy link

oakkitten commented Mar 10, 2019

what i suggested was not whitelisting all commands but a small number of handpicked commands that are surely safe, such as /query, /me, /msg etc, that an average relay client would use. the main idea is to make this option actually useful

when an evildoer is able to execute relay commands, they are either a

  1. mitm who is intercepting an insecure connection,
  2. someone who gained access to the relay password, or
  3. someone who has access to a device that runs a relay such as a phone.

in the latter case the owner of the server can change the password as they will surely notice a missing phone, so we can ignore that. that leaves us with an evildoer that can do 1. or 2.—someone who is pretty good at what they are doing. so if you want to protect the relay server, it doesn't make much sense to just blacklist some of the most obvious dangerous weechat commands. you'd have to go through all commands and carefully examine them, and also scripts; that would take a great amount of time and some things will probably get missed anyway. so a blacklist is a poor solution

a whitelist by default would be much easier to do right.

(for weechat-android, one caveat is that it uses the /input set_unread_current_buffer command, so you'd probably have to whitelist /input, but /input itself can be used to run arbitrary commands)

@demure
Copy link

demure commented Mar 10, 2019

If I'm reading the current relay-config.c right, it looks like /trigger is not currently blacklisted.
Depending on how much we want to default black, trigger can do an /exec so it may be worth considering. Though typing a valid trigger on a phone will probably be arduous.

flashcode added a commit that referenced this issue Mar 11, 2019
…issue #928)

The relay client is supposed to be safe by default, and the relay connection
should be protected by the different ways (restriction on IP address, SSL,
strong password, Time-based One-Time Password, local bind address and use of
SSH tunnel…).

So this option lets the user add extra security by allowing only some
commands (whitelist), or allowing any commands except a list of given
commands (blacklist).
flashcode added a commit that referenced this issue Mar 11, 2019
…928)

The whole command with arguments and the full buffer name are now displayed in
the warning message (in debug mode only).
@flashcode
Copy link
Member

In the latest commits related to this issue, I finally removed the default value in the option, so all commands are allowed by default (like in the latest stable version).
The user can use the option to customize the allowed/rejected commands (whitelist or blacklist).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request
Projects
None yet
Development

No branches or pull requests