Shell Command injection in ufw_backend.py

[Bernd Dietzel]

Firewall Administrators can be tricked by someone to export a profile with Gufw to an special crafted file or path name wich contains shell code.

reason is this line in ufw_backend.py :

def export_profile(self, profile, file):
    commands.getstatusoutput('cp /etc/gufw/' + profile + '.profile ' + file + ' ; chmod 777 ' + file)

The rename and delete funktions are also unsave if profile name contains shell code, like semicolons.

[costales (costales)]

Hi! Thanks a lot for your feedback!

The user only can to create profiles with letters, numbers, dashes and
underscores.
http://bazaar.launchpad.net/~costales/gui-ufw/gufw-15.04/view/head:/gufw/view/preferences.py#L101
<http://bazaar.launchpad.net/%7Ecostales/gui-ufw/gufw-15.04/view/head:/gufw/view/preferences.py#L101>
A profile with semicolons will be reject.

I was thinking about to filter in the ufw_backend.py in any way?
Best regards!

[Bernd Dietzel (l-ubuntuone1104)]

I managed to start xterm (2x) with root rights when the firewall admin exports the profile to a path like

/.../.../;xterm;/.../

so the profile name is not the key but the path in the "file" variable(s) in the export function .

For a workaround the export path should not be user editable but a fixed standard path an filename.

[costales (costales)]

Hi! I was thinking about this...

I think, as you said, the vulnerability could be the import/export by
the path. I added these lines for check that:
=== modified file 'gufw/view/gufw.py'
--- gufw/view/gufw.py 2014-12-13 15:33:17 +0000
+++ gufw/view/gufw.py 2015-01-16 15:51:03 +0000
@@ -344,6 +344,12 @@

      def on_menu_import_activate(self, widget, data=None):
          import_profile = self._file_dialog('open', _("Import Profile"))
+
+ # Shell injection?
+ if not os.path.exists(import_profile):
+ self.show_dialog(self.winMain, _("Path not valid"),
_("Please, report a bug here http://bugs.launchpad.net/gui-ufw"))
+ return
+
          profile = os.path.basename(import_profile) #Filename
          profile = os.path.splitext(profile)[0] # Ext

@@ -367,6 +373,11 @@
      def on_menu_export_activate(self, widget, data=None):
          export_profile = self._file_dialog('save', _("Export Profile"))

+ # Shell injection?
+ if not os.path.exists(export_profile):
+ self.show_dialog(self.winMain, _("Path not valid"),
_("Please, report a bug here http://bugs.launchpad.net/gui-ufw"))
+ return
+
          if not export_profile:
              self.set_statusbar_msg(_("Export cancelled"))
              return

In other way, I think the profile name can't give a Shell injection,
because the init profiles are read as regular files from
/etc/gufw/*.profile, a new profile will be check this pattern:
^[A-Za-z0-9_-]*$ and a deleted profile will have the previous patter or
it will be a regular file.

What do you think? :)

Thanks a lot for your awesome feedback!
Costales

[costales (costales)]

Need test before a release!

[Bernd Dietzel (l-ubuntuone1104)]

hmmm...
would it not be easyer to have a "backup" button instead of export ?
If the user clicks "backup" button he gets a message like this

"A backup of your Office.profile has bes saved to /etc/gufw/backup/Office_(Time and Date here).profile"
"Do you want to open the backup folder in your file browser now ? yes/NO"

And if he clicks "YES" , the file browser (nautilus) opens with user rigths leading him to the fixed path /etc/gufw/backup/ where he can copy and paste his backups to an other path if he wants.

So the export function can have a fixed path in his string without user interaction.

[costales (costales)]

Hi Bernd,
The backup button is really a good idea! but...

1. The users will ask about come back the import/export.
2. It's more complicate restore a backup between machines (in local machine it could be Backup/Restore and show just the backup profiles). Yes, I know is easy, but for some users not.
3. I'd prefer not to open Nautilus, it will be a root window.

In other way, in my tests, the imports and exports didn't allow something like this: /.../.../;xterm;/.../
Which environment are you using? I'm using Xubuntu and when I inserted the path /.../.../;xterm;/.../ and click "Open", the file browser is going to / path and I have to select always a file.

Best regards!

[Bernd Dietzel (l-ubuntuone1104)]

The backup/restore could be a nice feature to have a way back if the user does something wrong.
If he presses restore (import) he could find the standard profiles, wich Gufw had at first run , in the backup folder.
Gufw would be able to make automated backups, too.

I like the file browser with user rights too, not root rights, i said that in my comment above ;-)

My environment :
I am using Ubuntu Mate 14.10 , 32 bit, with kernel 3.16.0-24-generic

I made a (german) proof of concept Video where you can see that the bug works on my mashine.
https://www.youtube.com/watch?v=Kspdl_3TKG8

[costales (costales)]

Hi!

Your video was really clear! Thanks!!

About the change "Import/Export" > "Backup/Restore" is another improvement. I'll think on it ;) But I'd prefer an import/export.

Bernd, Could you confirm me if the attachment patch is fixing the injection?
It's working for me in the python Shell and I'll try in a few hours with Gufw in my computer.

If it's fixed, I'll send to the repository maintainers.

Thanks a lot for report this vulnerability!!!

[Bernd Dietzel (l-ubuntuone1104)]

As i want to test the patch , gufw did not start at all.
Seem because in my language there is no "Public.profile" , but in german language it's "Öffentlich.profile".

Here is the error message when i start gufw from a terminal:

Traceback (most recent call last):
  File "/usr/share/gufw/gufw/gufw.py", line 27, in <module>
    controler = Controller()
  File "/usr/share/gufw/gufw/controller.py", line 23, in __init__
    self.frontend = Frontend()
  File "/usr/share/gufw/gufw/model/frontend.py", line 23, in __init__
    self.firewall = Firewall()
  File "/usr/share/gufw/gufw/model/firewall.py", line 43, in __init__
    self._user_changed_language() # Rename profile files
  File "/usr/share/gufw/gufw/model/firewall.py", line 314, in _user_changed_language
    self.backend.rename_file_profile('Public', _("Public"))
  File "/usr/share/gufw/gufw/model/ufw_backend.py", line 270, in rename_file_profile
    os.rename(src, dst)
OSError: [Errno 2] Datei oder Verzeichnis nicht gefunden

This OSError means "file not found" in german language.
So i print the src and dst variables

src = '/etc/gufw/Public.profile'
dst= '/etc/gufw/\xc3\x96ffentlich.profile'

as you can see, there are Problems with the german letter "Ö" in the profile name "Öffentlich.profile"

[Bernd Dietzel (l-ubuntuone1104)]

or the Problem is that the "Public.profile" does not exist at all in my /etc/gufw/ folder because it was already renamed ?

[costales (costales)]

Uhm... Did you change your language? Do you have the 3 default profiles?

[Bernd Dietzel (l-ubuntuone1104)]

My default language is german.
i have 4 files in /etc/gufw/

gufw.cfg
Öffentlich.profile
Office.profile
Zu_Hause.profile

Öffentlich means "Public" in german language.
Zu_Hause means "At Home" in german language.
By the way ... "Office".profile should have the name "Büro.profile".

I think the problem ist that os.rename can not rename a file that no longer exists because it is already renamed.
There should be a "If the file Public.profile exists then rename it" in the code before renaming it at all.

-----

Meanwhile i commentet out the renaming stuff and gufw starts normaly ,
so i was able to test if the shell command injection problem i still there or not,

the code seems to be OK now .... great !

[costales (costales)]

:P I think you found another bug xD A lot of work this weekend :P But I really like users like you, reporting feedbacks/issues/bugs/vulnerabilities! :)

[Bernd Dietzel (l-ubuntuone1104)]

upps ... :-)
Have a nice weekend and thank you for the fix

[costales (costales)]

Fixed! I wish :P

Please, could you try the next patch?

It works now, the Office profile is empty in the translation, but it'll enter this night with a new translations import.

It's working right in English, if you confirm me it is working all perfect now, I'll request the upload to the repository.

Thanks a lot and have a nice weekend too |o/

[costales (costales)]

Please, if you can, try the final patch (it's the both patchs in this bug, together).

You will need to restart ufw_frontend.py to the default code, the easy way is:
sudo apt-get purge gufw && sudo apt-get install gufw
Then, apply the patch to ufw_frontend.py.

Thanks in advance!

[Bernd Dietzel (l-ubuntuone1104)]

i will try it soon

[Bernd Dietzel (l-ubuntuone1104)]

OK, works fine !

the try and except was an good idea :-)

I have not purged the gufw because my mate-desktop would be also purged when i would do that.
It was not nessesary to purge , patches works , no shell command injection any more, no renaming problems.

Thank you !

[costales (costales)]

I will send it tomorrow then!!
Really, thank you!!! Awesome issue! :)
Cheers
On Jan 17, 2015 9:10 PM, "Bernd Dietzel" <email address hidden> wrote:

> OK, works fine !
>
> the try and except was an good idea :-)
>
> I have not purged the gufw because my mate-desktop would be also purged
> when i would do that.
> It was not nessesary to purge , patches works , no shell command injection
> any more, no renaming problems.
>
> Thank you !
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1410839
>
> Title:
> Shell Command injection in ufw_backend.py
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/gui-ufw/+bug/1410839/+subscriptions
>

[costales (costales)]

@Martin, @devid: Could you upload the patch archive (patchs.tar.gz) to Ubuntu 14.04 & 14.10?

I'll send a new version tomorrow for Ubuntu 15.04.

Thanks in advance!!!

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "path_1410839.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[costales (costales)]

@Bernd: All is done :) I sent just now the updated version 15.04.1.
I want to thank you the report of an impotant vulnerability like this |o/ Thanks!!

[costales (costales)]

Updated patchs (it crashed with no profiles = first run).

[Bernd Dietzel (l-ubuntuone1104)]

Interessiting. One thing leads to an other thing :-)

If its get's worse you may wan't to think about going back and using subprocess.popen() instead of the old commands.getstatusoutput()

This could make the code shorter.

[costales (costales)]

@Bernd, I owe you a beer ;P
I was reviewing the code and I found another shell injection in the IP & Ports :(
I'm attaching the patchs for all the affected versions and I'm sending the new version 15.04.2 to the maintainers.
Best regards and thanks Bernd!

[Bernd Dietzel (l-ubuntuone1104)]

It was an honor to help you :-)

Maybe it would be an good idea to think about 'quoting' each and every parameter before it's passed to command ?
https://docs.python.org/3/library/shlex.html#shlex.quote

with best reagrds
Bernd

[Bernd Dietzel (l-ubuntuone1104)]

I was able to use "iface" to insert a shell command, too.

1.) save a profile wich uses some interface , for example "eth0" to your home directory.
2.) edit the file like this

iface = eth0;xterm;

3.) rename the profile to some other name than before
4.) import the new profile with Gufw from your home directory
5.) use the new profile
6.) xterm starts .... boom :-)

[costales (costales)]

Wow Bernd! :) You're doing a really awesome review!!
I'll be in paranoiac mode on and I'll check all the parameters.
Please, take a look to the path :) Thanks in advance!

[Bernd Dietzel (l-ubuntuone1104)]

Ok, the parameters are filtered now.

I'd still like to see subprocess.Popen() in combination with it's Parameter shell=False in the code.
Please, do not use commands.getstatusoutput() , its unsave when there are arguments in the string wich the attacker can reach.
Subprocess.Popen() directs the arguments in a better way to the program you want to run , so the args can not execute an other program.
https://docs.python.org/2/library/subprocess.html

And again, think about "quoting" if you still want to use commands.getstatusoutput() for some reason.
Quoting with shlex.quote(arg) should prevent shell command injection and ...
Quoting may also prevent an attacker to disable the firewall if he appends some valid ufw commands, not only shell commands ;-)
https://docs.python.org/3/library/shlex.html#shlex.quote

Greetings from germany
Bernd

[costales (costales)]

Hi Bernd!

Yes, you are right. I tried subproccess a few years ago and I found
something that was not working in what I need (i don't remember what). But
I will try it again :) I will create another bug for that and I will give
you a feedback.

I can't upload that change because It'll be complicate to asure the current
GUI stability and for older versions I have to fix problems but I must not
to make improvements.

In other way, this bug was epic. I learned a lot about (not web PHP)
injection. I want to thank you all the reports, tests and help!!! :) Really
thank you!!

Best regards!!

[Launchpad Janitor (janitor)]

This bug was fixed in the package gui-ufw - 15.10.0-0ubuntu1

---------------
gui-ufw (15.10.0-0ubuntu1) wily; urgency=medium

  * New upstream release. Upstream changelog:
    + 15.10.0
      - Added miniDLNA profile
      - Updated languages
    + 15.04.4
      - Fix: Migrate commands to subprocess > Fixing shell injection (LP: #1412554)
      - Fix: Allow import profile with English language (LP: #1416631)
      - Removed executable flag in config files (mask 600, not 700)
      - Updated translations
    + 15.04.3
      - Properly fix: Shell Command Injection (LP: #1410839)
    + 15.04.2
      - Fix: Shell Injection in the IP & Ports values.
    + 15.04.1
      - Fix: Shell Command Injection (LP: #1410839)
      - Fix: Not allow one interface over the same interface (LP: #1402220)
      - Fix: Not allow Both Protocol with a range of ports (LP: #1402232)
      - Updated languages
  * debin/control: bump Standard-Version to 3.9.6.

 -- Devid Antonio Filoni <email address hidden> Thu, 04 Jun 2015 21:01:39 +0200

[Chris J Arges (arges)]

Hello Bernd, or anyone else affected,

Accepted gui-ufw into vivid-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gui-ufw/15.04.4-0ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Ubuntu Foundations Team Bug Bot (crichton)]

The fix for this bug has been awaiting testing feedback in the -proposed repository for vivid for more than 90 days. Please test this fix and update the bug appropriately with the results. In the event that the fix for this bug is still not verified 15 days from now, the package will be removed from the -proposed repository.

[Bernd Dietzel (l-ubuntuone1104)]

fix works.