Shell Code Injection in hsi backend

[Bernd Dietzel]

https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103

The "hsi" backend of duplicity is vulnerabe to code injections.

It uses os.popen3() with should be replaced with subprocess.Popen().

Thank you.

File :
-------
/usr/lib/python2.7/dist-packages/duplicity/backends/hsibackend.py

This is the function witch is vulnerable :
------------------------------------------------------------
    def _list(self):
        commandline = '%s "ls -l %s"' % (hsi_command, self.remote_dir)
        l = os.popen3(commandline)[2].readlines()[3:]

Exploit Demo :
============

On the Terminal type in :

$ duplicity 'hsi://bug/";xeyes;"/test/' /tmp/bug

--> This will start the program xeyes , but should not.

I attached a screenshot of the exploit demo.

[Kenneth Loafman (kenneth-loafman)]

This has been fixed in the trunk version which Ubuntu won't use because it's new and follows proper version numbering conventions. No code in duplicity uses popen3() in recent versions.

[Kenneth Loafman (kenneth-loafman)]

Eating crow now. HSI backend did use popen3(). I'll have a fix out in a bit, but am unable to test it, no HSI access.

[Kenneth Loafman (kenneth-loafman)]

Fixed as suggested.

Now to figure out why grep could not find 'popen3'. AArgh!

[Bernd Dietzel (l-ubuntuone1104)]

Hello Kenneth,
i think you use subprocess in a insecure way and the patch will not work.

1) you use shell=True in backend.py in this function :

    def __subprocess_popen(self, commandline):
        """
        For internal use.
        Execute the given command line, interpreted as a shell command.
        Returns int Exitcode, string StdOut, string StdErr
        """
        from subprocess import Popen, PIPE
        p = Popen(commandline, shell=True, stdout=PIPE, stderr=PIPE)
        stdout, stderr = p.communicate()

        return p.returncode, stdout, stderr

2) you do not separate the command arguments as a list object before passing them to subprocess.
This opens the way for hackers to put any strings and arguments into the commandline.

3) So, please do it more in a way like this (havend testet it, but you will know what i mean) :

    commandline = [ hsi_command, 'ls', '-l', self.remote_dir ]

Thanks :-)

[Bernd Dietzel (l-ubuntuone1104)]

OK, i could verify that we have second bug, its in backend.py where subprocess is called.

This affects ALL scripts using the __subprocess_popen function.

hsibackend.py , lftpbackend.py , ncftpbackend.py , rsyncbackend.py , sxbackend.py , tahoebackend.py

Exploit demo :
===========

$ duplicity /tmp/ 'rsync://user@host//;xmessage hello bug;#/test'

[Kenneth Loafman (kenneth-loafman)]

Reopened and added duplicity-team to the subscribers. This is not as simple as splitting the command line into parts. As long as you can make the the pathname contain ';', you can insert commands. Given the example you gave last," ;xmessage hello bug;#/test" is a valid filename, so filename validation will not work.

[Bernd Dietzel (l-ubuntuone1104)]

Not only filenames, ... anything in the path like the name of a usb-stick could get a shell command.

So please to not use "shell=True" , it makes the "subprocess.Popen" command as insecure as the "os.popen" commands are.

[Aaron Whitehouse (aaron-whitehouse)]

Looks like we should move away from shell=True ASAP.

https://docs.python.org/2/library/subprocess.html#frequently-used-arguments
"Warning
Executing shell commands that incorporate unsanitized input from an untrusted source makes a program vulnerable to shell injection, a serious security flaw which can result in arbitrary command execution. For this reason, the use of shell=True is strongly discouraged in cases where the command string is constructed from external input [...]"

[edso (ed.so)]

On 29.11.2015 17:34, Aaron Whitehouse wrote:
> Looks like we should move away from shell=True ASAP.
>
> https://docs.python.org/2/library/subprocess.html#frequently-used-arguments
> "Warning
> Executing shell commands that incorporate unsanitized input from an untrusted source makes a program vulnerable to shell injection, a serious security flaw which can result in arbitrary command execution. For this reason, the use of shell=True is strongly discouraged in cases where the command string is constructed from external input [...]"
>

that's going to be some effort. we have several backends utilizing shell binaries and at least ftp has a similar issue.

btw. how is ist the GpgInterface dealing with this?
 https://bazaar.launchpad.net/~duplicity-team/duplicity/0.7-series/view/head:/duplicity/gpginterface.py
theoretically it should have a similar attack surface but it seems to utilize os.execvp(command[0], command) which seems to have no shell injection issues, as no shell is used.
this guy seems to have dug up how subprocess.call() actually uses os.execvp() internally.
 http://blog.littleimpact.de/index.php/2008/08/11/avoiding-shell-injection-in-ruby-python-and-php/

in summary i'd agree that switching shell=False for subprocess calls should do the trick.
ages ago i tried to streamline subprocess usage by adding Backend.subprocess_popen(). reworking all backends to use it and just fix it there should suffice. obviously it will need to be given a list of arguments with a fixed arg[0] run command in that case ;)

..ede/duply.net

[Kenneth Loafman (kenneth-loafman)]

Just committed a fix that should resolve all of these issues.

[Bernd Dietzel (l-ubuntuone1104)]

I'm not happy with shlex.split.
It could split the commandline into unwanted additional arguments.

Example with a unwanted help argument in a path :

theregrunner@PC:~$ python
Python 2.7.10 (default, Oct 14 2015, 16:09:02)
[GCC 5.2.1 20151010] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> commandline = '%s "ls -l %s"' % ('program' , '/tmp/" "--help')
>>> commandline
'program "ls -l /tmp/" "--help"'
>>> import shlex
>>> args = shlex.split(commandline)
>>> args
['program', 'ls -l /tmp/', '--help']
>>>

[edso (ed.so)]

On 30.11.2015 18:45, Bernd Dietzel wrote:
> I'm not happy with shlex.split.
> It could split the commandline into unwanted additional arguments.
>
> Example with a unwanted help argument in a path :
>
> theregrunner@PC:~$ python
> Python 2.7.10 (default, Oct 14 2015, 16:09:02)
> [GCC 5.2.1 20151010] on linux2
> Type "help", "copyright", "credits" or "license" for more information.
>>>> commandline = '%s "ls -l %s"' % ('program' , '/tmp/" "--help')
>>>> commandline
> 'program "ls -l /tmp/" "--help"'
>>>> import shlex
>>>> args = shlex.split(commandline)
>>>> args
> ['program', 'ls -l /tmp/', '--help']
>>>>

can you see a security implication? worst case afaics is breakage.

 actually backends had to have the parameters properly quoted already. if not they wouldn't work with spaces or other special chars. not sure that all were tested against these corner cases though.

..ede/duply.net

[Kenneth Loafman (kenneth-loafman)]

I needed arg[0] to replace partial path with full path and shlex was the
only way to do this easily.

It also parses the commandline the way the shell would, so a legal split.
Your example will be reconstructed in Popen as

"program ls -l /tmp --help"

which is weird, but probably legal in most cases.

I thought about doing it with a simple split(), but that would run out
something like

'/usr/bin/odd dir with spaces/program' ls /tmp

so I went with something that is supposed to work.

Basically, the problem goes from simple to absurdly complex in a real hurry.

On Mon, Nov 30, 2015 at 12:18 PM, edso <email address hidden> wrote:

> On 30.11.2015 18:45, Bernd Dietzel wrote:
> > I'm not happy with shlex.split.
> > It could split the commandline into unwanted additional arguments.
> >
> > Example with a unwanted help argument in a path :
> >
> > theregrunner@PC:~$ python
> > Python 2.7.10 (default, Oct 14 2015, 16:09:02)
> > [GCC 5.2.1 20151010] on linux2
> > Type "help", "copyright", "credits" or "license" for more information.
> >>>> commandline = '%s "ls -l %s"' % ('program' , '/tmp/" "--help')
> >>>> commandline
> > 'program "ls -l /tmp/" "--help"'
> >>>> import shlex
> >>>> args = shlex.split(commandline)
> >>>> args
> > ['program', 'ls -l /tmp/', '--help']
> >>>>
>
> can you see a security implication? worst case afaics is breakage.
>
> actually backends had to have the parameters properly quoted already.
> if not they wouldn't work with spaces or other special chars. not sure
> that all were tested against these corner cases though.
>
> ..ede/duply.net
>
> --
> You received this bug notification because you are subscribed to
> Duplicity.
> https://bugs.launchpad.net/bugs/1520691
>
> Title:
> Shell Code Injection in hsi backend
>
> Status in Duplicity:
> Fix Committed
>
> Bug description:
> https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
>
> The "hsi" backend of duplicity is vulnerabe to code injections.
>
> It uses os.popen3() with should be replaced with subprocess.Popen().
>
> Thank you.
>
> File :
> -------
> /usr/lib/python2.7/dist-packages/duplicity/backends/hsibackend.py
>
> This is the function witch is vulnerable :
> ------------------------------------------------------------
> def _list(self):
> commandline = '%s "ls -l %s"' % (hsi_command, self.remote_dir)
> l = os.popen3(commandline)[2].readlines()[3:]
>
> Exploit Demo :
> ============
>
> On the Terminal type in :
>
> $ duplicity 'hsi://bug/";xeyes;"/test/' /tmp/bug
>
> --> This will start the program xeyes , but should not.
>
> I attached a screenshot of the exploit demo.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/duplicity/+bug/1520691/+subscriptions
>

[Bernd Dietzel (l-ubuntuone1104)]

@edso
This depends on the program witch we call.
We can not check all of the possible parameter combinations if they lead to a leak.
So we do not want to have the arguments out of our hands.

@Kenneth
Why do we put the commands into a long string and afterwards "hopefully" spilt them again ?
We should put them directly into a commandlist at the point we still know what was what.
This would make it easy again ;-)

[edso (ed.so)]

On 30.11.2015 20:34, Bernd Dietzel wrote:
> @edso
> This depends on the program witch we call.
> We can not check all of the possible parameter combinations if they lead to a leak.
> So we do not want to have the arguments out of our hands.

well, most (if not all) legacy command lines start with a word, so parameter issues sound merely academic from a security point of view.

having written that i realise, there might be corner cases leading to sensitive files read/overwritten files when run as root, when the client binary fails to parse params correctly, that might be worth the effort to manually patch each and every backend affected.

> @Kenneth
> Why do we put the commands into a long string and afterwards "hopefully" spilt them again ?
> We should put them directly into a commandlist at the point we still know what was what.
> This would make it easy again ;-)
>

there is the "ominous" we agn. ;) patching every backend is a lot of effort and some "ominous" somebody needs to find the time to hack the solution. are you volunteering?

..ede/duply.net

[Bernd Dietzel (l-ubuntuone1104)]

@edso
> ... so parameter issues sound merely academic from a security point of view. ...

Not so academic as you think , i could for example exploit the program Gufw with the legal parameter "disable" so the firewall went off, witch was not wanted and not shown in the gui.

> ... there is the "ominous" we agn. ;) ....

I used "we should ... " because it sounds so hard if i say "you have made some mistake" ... ;-)
I can help patching, but i found more than 30 Shell Injections in other python scripts , so ... you are not the only ones ;-)
My buglist where you can find some inspiration how the other ones fixed their bugs
https://bugs.launchpad.net/~l-ubuntuone1104/+bugs?orderby=-importance&start=0

[Kenneth Loafman (kenneth-loafman)]

The way it is implemented now will probably not be exploitable and will
probably work correctly as long as users behave rationally. Exploits
depending on multiple statements being executed in one command will not
work. Other exploits are possible, I'm sure, but not through shell code
injection.

The whole idea of shell code injection implies bad player access, which is
the issue that should be most important to close.

On Mon, Nov 30, 2015 at 3:11 PM, Bernd Dietzel <email address hidden>
wrote:

> @edso
> > ... so parameter issues sound merely academic from a security point of
> view. ...
>
> Not so academic as you think , i could for example exploit the program
> Gufw with the legal parameter "disable" so the firewall went off, witch
> was not wanted and not shown in the gui.
>
> > ... there is the "ominous" we agn. ;) ....
>
> I used "we should ... " because it sounds so hard if i say "you have made
> some mistake" ... ;-)
> I can help patching, but i found more than 30 Shell Injections in other
> python scripts , so ... you are not the only ones ;-)
> My buglist where you can find some inspiration how the other ones fixed
> their bugs
>
> https://bugs.launchpad.net/~l-ubuntuone1104/+bugs?orderby=-importance&start=0
>
> --
> You received this bug notification because you are subscribed to
> Duplicity.
> https://bugs.launchpad.net/bugs/1520691
>
> Title:
> Shell Code Injection in hsi backend
>
> Status in Duplicity:
> Fix Committed
>
> Bug description:
> https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
>
> The "hsi" backend of duplicity is vulnerabe to code injections.
>
> It uses os.popen3() with should be replaced with subprocess.Popen().
>
> Thank you.
>
> File :
> -------
> /usr/lib/python2.7/dist-packages/duplicity/backends/hsibackend.py
>
> This is the function witch is vulnerable :
> ------------------------------------------------------------
> def _list(self):
> commandline = '%s "ls -l %s"' % (hsi_command, self.remote_dir)
> l = os.popen3(commandline)[2].readlines()[3:]
>
> Exploit Demo :
> ============
>
> On the Terminal type in :
>
> $ duplicity 'hsi://bug/";xeyes;"/test/' /tmp/bug
>
> --> This will start the program xeyes , but should not.
>
> I attached a screenshot of the exploit demo.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/duplicity/+bug/1520691/+subscriptions
>

[edso (ed.so)]

On 30.11.2015 23:05, Kenneth Loafman wrote:
> The whole idea of shell code injection implies bad player access, which is
> the issue that should be most important to close.

we backup to potentially insecure backends. that's what the encryption is for ;).. in theory a malicious party could fiddle with the file names on the backend and i am pretty sure no one tested this possibility with shell based backends so far wrt. shell injections.

so actually, disagreed. but as usual, it will be done if somebody does it ;) not earlier.

btw. python pexpect seems to shlex.split() too if it isn't provided a list of arguments
 https://github.com/pexpect/pexpect/blob/master/pexpect/popen_spawn.py#L42

..ede/duply.net

[edso (ed.so)]

On 30.11.2015 22:11, Bernd Dietzel wrote:
> @edso
>> ... so parameter issues sound merely academic from a security point of view. ...
>
> Not so academic as you think , i could for example exploit the program Gufw with the legal parameter "disable" so the firewall went off, witch was not wanted and not shown in the gui.

provided you manage "Gufw" to be executed which at least now is much harder, maybe impossible, would have to check all backends to determine.

>> ... there is the "ominous" we agn. ;) ....
>
> I used "we should ... " because it sounds so hard if i say "you have made some mistake" ... ;-)

well, how would you know if we wrote that part anyhow ;) might have been the original author or some contributor years ago. in these cases i like to stick the matter at hand. eg. "there is a mistake/error because"

the "ominous" we is like it's siblings.. this "no one" who is responsible and the "some one" who should do something about it. we should be used if you are a part of that group and only then ;)

> I can help patching, but i found more than 30 Shell Injections in other python scripts , so ... you are not the only ones ;-)

at least you understand that we are a very small team with jobs and this spare-time activity, so if something does not happen instantly or at all that's usually because the lack of time.

> My buglist where you can find some inspiration how the other ones fixed their bugs
> https://bugs.launchpad.net/~l-ubuntuone1104/+bugs?orderby=-importance&start=0
>

thanks, will have a look.

..ede/duply.net

[Bernd Dietzel (l-ubuntuone1104)]

Hi edso,
It seems i found a shlex parameter example witch could get a problem.
Could you please check the rsync backend for what happens when we have a rsync path like this ?

/tmp/" "--log-file=xxx

This may overwrite/create any local file , here with the name xxx , when the rsync backend is used.

Thanks ;-)

[Bernd Dietzel (l-ubuntuone1104)]

second test :

 use the path

/tmp/ --log-file=xxx

[edso (ed.so)]

On 01.12.2015 22:44, Bernd Dietzel wrote:
> second test :
>
> use the path
>
> /tmp/ --log-file=xxx
>

1. can you provide a proper command line that illustrates a problem? along the lines of 'duplicity /local/path rsync://'

2. this would be a simple bug, but no security issue. actually what you describe is legally possible with duplicity by using the parameter --rsync-options.

if you can come up w/ an attack where the filenames on the backend were maliciously modified in a way that exploits a locally run duplicity, than you'd have me convinced instantly.

..ede/duply.net

[Kenneth Loafman (kenneth-loafman)]

No attack, but it does something odd...

---- normal run ---
ken@stealth:~$ rm -f /tmp/testdup/*; duplicity full -v9 ~/bin
'rsync://stealth///tmp/testdup'
ken@stealth:~$ ll /tmp/testdup
total 72
-rw------- 1 ken ken 834 Dec 2 07:13
duplicity-full.20151202T131324Z.manifest.gpg
-rw------- 1 ken ken 56942 Dec 2 07:13
duplicity-full.20151202T131324Z.vol1.difftar.gpg
-rw------- 1 ken ken 10628 Dec 2 07:13
duplicity-full-signatures.20151202T131324Z.sigtar.gpg

--- funny run ---
ken@stealth:~$ rm -f /tmp/testdup/*; duplicity full -v9 ~/bin
'rsync://stealth///tmp/testdup --log-file=xxx'
ken@stealth:~$ ll /tmp/testdup
total 72
-rw------- 1 ken ken 833 Dec 2 07:16
duplicity-full.20151202T131620Z.manifest.gpg
-rw------- 1 ken ken 10628 Dec 2 07:16
duplicity-full-signatures.20151202T131620Z.sigtar.gpg
-rw------- 1 ken ken 56942 Dec 2 07:16 mktemp-xaTcaB-2 <== contains the
missing difftar contents.

So, something is going on, but what is currently unknown. The xxx log file
was never created, so no attack. It's possible it may have been created in
one of the temp dirs, but not on the system.

On Wed, Dec 2, 2015 at 6:33 AM, edso <email address hidden> wrote:

> On 01.12.2015 22:44, Bernd Dietzel wrote:
> > second test :
> >
> > use the path
> >
> > /tmp/ --log-file=xxx
> >
>
> 1. can you provide a proper command line that illustrates a problem?
> along the lines of 'duplicity /local/path rsync://'
>
> 2. this would be a simple bug, but no security issue. actually what you
> describe is legally possible with duplicity by using the parameter
> --rsync-options.
>
> if you can come up w/ an attack where the filenames on the backend were
> maliciously modified in a way that exploits a locally run duplicity,
> than you'd have me convinced instantly.
>
> ..ede/duply.net
>
> --
> You received this bug notification because you are subscribed to
> Duplicity.
> https://bugs.launchpad.net/bugs/1520691
>
> Title:
> Shell Code Injection in hsi backend
>
> Status in Duplicity:
> Fix Committed
>
> Bug description:
> https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
>
> The "hsi" backend of duplicity is vulnerabe to code injections.
>
> It uses os.popen3() with should be replaced with subprocess.Popen().
>
> Thank you.
>
> File :
> -------
> /usr/lib/python2.7/dist-packages/duplicity/backends/hsibackend.py
>
> This is the function witch is vulnerable :
> ------------------------------------------------------------
> def _list(self):
> commandline = '%s "ls -l %s"' % (hsi_command, self.remote_dir)
> l = os.popen3(commandline)[2].readlines()[3:]
>
> Exploit Demo :
> ============
>
> On the Terminal type in :
>
> $ duplicity 'hsi://bug/";xeyes;"/test/' /tmp/bug
>
> --> This will start the program xeyes , but should not.
>
> I attached a screenshot of the exploit demo.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/duplicity/+bug/1520691/+subscriptions
>

[Bernd Dietzel (l-ubuntuone1104)]

Ok, found why it not works.
The character "/" ist automatically added at the end, so it results in "--log-file=xxx/" wtich wont work.
If some valid parameter is at the end witch likes the "/" added, it works.
In this Demo, i added "--partial-dir=/tmp" witch gets to "--partial-dir=/tmp/" witch i s valid.
The xxx file was created in my home folder.

duplicity 'rsync://127.0.0.1/bug/ --log-file=xxx --partial-dir=/tmp' /home/Downloads/

So, when i use the rsync backend, any parameter witch allows to have a "/" at the end will be executed.

[edso (ed.so)]

On 02.12.2015 17:48, Bernd Dietzel wrote:
> Ok, found why it not works.
> The character "/" ist automatically added at the end, so it results in "--log-file=xxx/" wtich wont work.
> If some valid parameter is at the end witch likes the "/" added, it works.
> In this Demo, i added "--partial-dir=/tmp" witch gets to "--partial-dir=/tmp/" witch i s valid.
> The xxx file was created in my home folder.
>
> duplicity 'rsync://127.0.0.1/bug/ --log-file=xxx --partial-dir=/tmp'
> /home/Downloads/
>
> So, when i use the rsync backend, any parameter witch allows to have a
> "/" at the end will be executed.
>

whilst imperfect, i will not spend time fixing this obvious flaw. as i wrote, the same can be achieved "properly" by using --rsync-options. also using rsync plainly with these arguments would have an identical result.

the only way to fix this is to patch each and every backend and have it shlex/pipes.quote() each and every string we use in the cmd line. but until i see a vulnerability springing from this issue i am not going to invest the effort, speaking only for myself here of course ;)

..ede/duply

[Kenneth Loafman (kenneth-loafman)]

The real problem is that this works since it's all valid pathname chars:
    echo foo > ' --log-file=xxx --partial-dir='
which means that
    /bug/ --log-file=xxx --partial-dir=/tmp
is a quite valid filename.

So, Catch-22, allow all valid filenames, or restrict somehow?

I'm going with allow all valid filenames.

BTW, you can remove with:
    rm ./' --log-file=xxx --partial-dir='

On Wed, Dec 2, 2015 at 11:28 AM, edso <email address hidden> wrote:

> On 02.12.2015 17:48, Bernd Dietzel wrote:
> > Ok, found why it not works.
> > The character "/" ist automatically added at the end, so it results in
> "--log-file=xxx/" wtich wont work.
> > If some valid parameter is at the end witch likes the "/" added, it
> works.
> > In this Demo, i added "--partial-dir=/tmp" witch gets to
> "--partial-dir=/tmp/" witch i s valid.
> > The xxx file was created in my home folder.
> >
> > duplicity 'rsync://127.0.0.1/bug/ --log-file=xxx --partial-dir=/tmp'
> > /home/Downloads/
> >
> > So, when i use the rsync backend, any parameter witch allows to have a
> > "/" at the end will be executed.
> >
>
>
> whilst imperfect, i will not spend time fixing this obvious flaw. as i
> wrote, the same can be achieved "properly" by using --rsync-options. also
> using rsync plainly with these arguments would have an identical result.
>
> the only way to fix this is to patch each and every backend and have it
> shlex/pipes.quote() each and every string we use in the cmd line. but
> until i see a vulnerability springing from this issue i am not going to
> invest the effort, speaking only for myself here of course ;)
>
> ..ede/duply
>
> --
> You received this bug notification because you are subscribed to
> Duplicity.
> https://bugs.launchpad.net/bugs/1520691
>
> Title:
> Shell Code Injection in hsi backend
>
> Status in Duplicity:
> Fix Committed
>
> Bug description:
> https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
>
> The "hsi" backend of duplicity is vulnerabe to code injections.
>
> It uses os.popen3() with should be replaced with subprocess.Popen().
>
> Thank you.
>
> File :
> -------
> /usr/lib/python2.7/dist-packages/duplicity/backends/hsibackend.py
>
> This is the function witch is vulnerable :
> ------------------------------------------------------------
> def _list(self):
> commandline = '%s "ls -l %s"' % (hsi_command, self.remote_dir)
> l = os.popen3(commandline)[2].readlines()[3:]
>
> Exploit Demo :
> ============
>
> On the Terminal type in :
>
> $ duplicity 'hsi://bug/";xeyes;"/test/' /tmp/bug
>
> --> This will start the program xeyes , but should not.
>
> I attached a screenshot of the exploit demo.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/duplicity/+bug/1520691/+subscriptions
>

[Bernd Dietzel (l-ubuntuone1104)]

Oh... Oh ...

duplicity /tmp 'rsync://localhost/ --rsh="firefox theregrunner" --backup-dir='

[Bernd Dietzel (l-ubuntuone1104)]

This is why i do not like to give the arguments out of my hands.
A parameter may start any program, like rsync starts firefox or xmessage :

duplicity 'rsync://x/ --rsh="xmessage "' ~/t

[Kenneth Loafman (kenneth-loafman)]

When you build the commandline in duplicity you have full control of what
you put in. Not sure what you mean.

On Wed, Dec 2, 2015 at 2:01 PM, Bernd Dietzel <email address hidden>
wrote:

> This is why i do not like to give the arguments out of my hands.
> A parameter may start any program, like rsync starts firefox or xmessage :
>
> duplicity 'rsync://x/ --rsh="xmessage "' ~/t
>
> --
> You received this bug notification because you are subscribed to
> Duplicity.
> https://bugs.launchpad.net/bugs/1520691
>
> Title:
> Shell Code Injection in hsi backend
>
> Status in Duplicity:
> Fix Committed
>
> Bug description:
> https://bugs.launchpad.net/ubuntu/+source/duplicity/+bug/1519103
>
> The "hsi" backend of duplicity is vulnerabe to code injections.
>
> It uses os.popen3() with should be replaced with subprocess.Popen().
>
> Thank you.
>
> File :
> -------
> /usr/lib/python2.7/dist-packages/duplicity/backends/hsibackend.py
>
> This is the function witch is vulnerable :
> ------------------------------------------------------------
> def _list(self):
> commandline = '%s "ls -l %s"' % (hsi_command, self.remote_dir)
> l = os.popen3(commandline)[2].readlines()[3:]
>
> Exploit Demo :
> ============
>
> On the Terminal type in :
>
> $ duplicity 'hsi://bug/";xeyes;"/test/' /tmp/bug
>
> --> This will start the program xeyes , but should not.
>
> I attached a screenshot of the exploit demo.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/duplicity/+bug/1520691/+subscriptions
>

[Bernd Dietzel (l-ubuntuone1104)]

The string

 --rsh="xmessage "

with a leading space is a valid path or filename , as you wrote in #27.

 When using shlex.split() in backend.py , this will get a arg for rsync which runs xmessage.

[edso (ed.so)]

On 02.12.2015 22:54, Bernd Dietzel wrote:
> The string
>
> --rsh="xmessage "
>
> with a leading space is a valid path or filename , as you wrote in #27.
>
> When using shlex.split() in backend.py , this will get a arg for rsync
> which runs xmessage.
>

yeah right. again, where is the security issue?

..ede/duply.net

[Bernd Dietzel (l-ubuntuone1104)]

The same issue as at the beginning.
But not a ";" will inject the command ... its a space now.

[edso (ed.so)]

not at all. rsync --rsh="binary" is a feature. if you are complaining that rsync runs another binary feel free to complain to the rsync devs. you should consider consulting rsync manpage first about what --rsh is meant for though.

and eventually - this is no exploit. it's merely a parsing bug which should of course be fixed, when time allows.

as i wrote earlier

"
if you can come up w/ an attack where the filenames on the backend were maliciously modified in a way that exploits a locally run duplicity, than you'd have me convinced instantly.
"

happy hunting.. ede/duply.net

[edso (ed.so)]

wrt. to your video.
 https://youtu.be/A5ol7bO_scQ

we are d'accord that there is a possibility that the parameter might contain an unwanted parameter. but as duplicity command lines are create by the user and _not_ an attacker it is in the user's purview to make sure the target url is proper.

there is no ui to my knowledge for duplicity that's browses a backend and let's the user pick a possibly malicious path.

having written all that - please come up with an attack based on the backends file naming
 or
please accept that this is going to stand as long as nobody finds time to tackle it
 or
ideally just fix it yourself and provide patches or a branch!

so long.. ede/duply.net

[Bernd Dietzel (l-ubuntuone1104)]

parsing bug using the lftp backend ?

Try lftp with a space " " or a ";" in the path :

duplicity /tmp/ "ftp://user@localhost/test /bug/"

duplicity /tmp/ ftp://user@localhost/test;bug/

[Bernd Dietzel (l-ubuntuone1104)]

maybe a problem when there is no trailig path delimiter it splits at the ";"

$ duplicity /tmp "ftp://user@localhost/test;bug"

commandline:
lftp -c 'source '/tmp/duplicity-0sMNWh-tempdir/mkstemp-h3m2mZ-1'; cd 'test/' || exit 0; ls'

args:
['/usr/bin/lftp', '-c', 'source /tmp/duplicity-0sMNWh-tempdir/mkstemp-h3m2mZ-1; cd test/ || exit 0; ls']

[Kenneth Loafman (kenneth-loafman)]

I think you are flogging a dead horse. As far as I can tell, it's not possible to detect intentional shell injection, and still allow all the chars in the filename that Linux does. You have some clever examples, but what's lacking is any suggestion on how to spot shell injections.

The lftp example is good, but that's in lftp itself. Since duplicity requires a path and a url, the commandline would be invalid.

[Bernd Dietzel (l-ubuntuone1104)]

Demo what may happen when someone does a copy and paste of an FTPS Path.
https://youtu.be/LeA6W3s-Q80

[edso (ed.so)]

On 27.12.2015 19:35, Bernd Dietzel wrote:
> Demo what may happen when someone does a copy and paste of an FTPS Path.
> https://youtu.be/LeA6W3s-Q80
>

of course you replicated the issue with the latest 0.7.06 release before complaining in length via video, right?

in 0.7.06 lftp simply chokes on it. feel free to open another bug to fix the lftp backend wrt that.

..ede/duply.net

[Bernd Dietzel (l-ubuntuone1104)]

Ok, thank you for testing.

I made a new demo to work with the newest Version ;-)

Find it here:
https://bugs.launchpad.net/duplicity/+bug/1529606