vinifmor / bauh

Graphical user interface for managing your Linux applications. Supports AppImage, Debian and Arch packages (including AUR), Flatpak, Snap and native Web applications
zlib License
959 stars 71 forks source link

Possible parameter injection in search query #266

Closed mijorus closed 2 years ago

mijorus commented 2 years ago

https://github.com/vinifmor/bauh/blob/8a81ae40985502f6d7353fdf8aff6520178547f2/bauh/commons/system.py#L215

Hi, I was checking the code and it looks like there isn't any sort of input sanification. In fact, if I run a search something like --user or --not-a-param it throws an error.

I don't know it this could have security implications, my guess is that it could as someone who knows the shell better than me might inject some code into it and run external commands using interpolation (?)

Or maybe I am wrong.

vinifmor commented 2 years ago

The search's command injection is possible indeed. Since bauh's input interface is an UI, it would require an attacker to gain full access to your desktop session and uses bauh to perform the attack (in this scenario, bauh would be your minor problem :) ). Considering the attack possibility and probability, I see this vulnerability as a a minor security issue.

Not all back-ends use this function to query, but I'm going to add a sanitization code anyway.

mijorus commented 2 years ago

more than injection, I would rather speak of unwanted sideeffects.

For example, searching for music -player is enough to make it crash, as -player is seen as an option.

I tested and for some reason flatpak does not like quotes, ex: flatpak search "music player" returns empty

vinifmor commented 2 years ago

Ok, could you attach logs so we can trace the crash cause ?

vinifmor commented 2 years ago

(the sanitization code is already on the staging branch -> https://github.com/vinifmor/bauh/commit/c0fde3686df747460993e84a8367479a564dba79)

mijorus commented 2 years ago

When searching for music -player

QSocketNotifier: Can only be used with threads started with QThread
error: Unknown option -player
Exception in thread Thread-68 (_search):
Traceback (most recent call last):
  File "/usr/lib/python3.10/threading.py", line 1009, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.10/threading.py", line 946, in run
    self._target(*self._args, **self._kwargs)
  File "/home/altravia/.local/lib/python3.10/site-packages/bauh/view/core/controller.py", line 144, in _search
    apps_found = man.search(words=word, disk_loader=disk_loader, is_url=is_url, limit=-1)
  File "/home/altravia/.local/lib/python3.10/site-packages/bauh/gems/flatpak/controller.py", line 103, in search
    apps_found = flatpak.search(flatpak.get_version(), words, remote_level)
  File "/home/altravia/.local/lib/python3.10/site-packages/bauh/gems/flatpak/flatpak.py", line 335, in search
    split_res = res.strip().split('\n')
AttributeError: 'NoneType' object has no attribute 'strip'
mijorus commented 2 years ago

This happens because -player is not recognised as an valid parameter, run_cmd fails and there is no try except

mijorus commented 2 years ago

267