search

qbittorrent / qBittorrent

qBittorrent BitTorrent client
https://www.qbittorrent.org
Other
28.2k stars 3.97k forks source link

Disallow/strip leading whitespace from command fields/config entries (used in Malware attack) #20932

Open r0ckarong opened 5 months ago

r0ckarong commented 5 months ago

qBittorrent & operating system versions

qBittorrent 4.6.5 from Docker image https://github.com/linuxserver/docker-qbittorrent/releases

What is the problem?

The "Run external program" fields (and any other settings field btw.) allow whitespace to precede the entered values. This is used by malicious actors to "hide" commands from the user at first glance.

I was targeted by crypto malware when I accidentally exposed my WebUI to the web. The attacker modified the configuration to enable "Run external program on torrent finished" and added a command that would fetch and install some crypto miner. Luckily this only happened inside a Docker container for me. I noticed this because there was some odd behavior of the client and high memory usage.

I noticed some weird activity in the log though and realized that these external commands ran.

When I checked the config, I initially didn't even notice it because how often do you look at your client config after you've set it? What was supposed to be there and what looked odd?

The way they "hide" this in the UI is to pad the command string with lots of whitespace (140+) so the actual command would "disappear". The provided input box has only a limited number of chars on screen and you have to actively select and look at it to figure out there is actually something in there. The ticked checkbox wasn't even obvious as weird to me when I looked over it the first couple of times.

The config file then confirmed that there was a space padded command in there.

I would like to request the input fields (and the storage in the config file for that matter) to be reworked in such a way that they do not allow whitespace to precede commands or inputs. I can't think of any reasonable case where you would need whitespace "before" any of the settings in the config file.

Steps to reproduce

  1. Open Settings > Downloads
  2. Tick box for "Run external program on torrent finished"
  3. Hit Space 140 Times
  4. Enter some malicious crap
  5. Hit "Home" and watch your command disappear (except for in the config)

Additional context

Other people hit by this: https://www.reddit.com/r/synology/comments/1co3toi/rogue_process_eating_ram/

(Sorry I don't have the config anymore but you can reproduce this yourself easily).

r0ckarong commented 5 months ago

Last night I thought underscores should probably be filtered out before these strings as well. They could blend into the input field frame and achieve the same effect than the whitespace.

HanabishiRecca commented 5 months ago

We could just strip whitespaces on save. It should be enough at least for WebUI case, I think.

HanabishiRecca commented 5 months ago

On the other hand, it does not address the actual problem. If a malicious actor managed to execute arbitrary code, it is already too late. Miners are relatively harmless scenario, it could be much worse.

Now I think giving user ability to set that fields via WebUI is too risky. IMO, the best course of action here is to remove them from the WebUI completely, and from the web API in general.

People running headless servers should figure out how to edit the config file manually.