Bug 28189 - durch UMC license import können Dateien gelöscht werden
durch UMC license import können Dateien gelöscht werden
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC - Domain management (Generic)
UCS 3.0
Other Linux
: P3 normal (vote)
: UCS 3.1
Assigned To: Florian Best
Dirk Wiesenthal
: interim-2
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-13 09:31 CEST by Florian Best
Modified: 2021-06-23 07:29 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Security Issue
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:
best: Patch_Available+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2012-08-13 09:31:34 CEST
Der Lizenzupload erlaubt das Hochladen der Lizenz (über einen HTTP Upload) und ein normaler json-request mit dem Inhalt der Lizenz.

Am Ende vom Lizenz-upload wird die "temporäre Lizenz"-Datei gelöscht.

Im Code wird nicht ausreichend geprüft, welcher Typ von upload verwendet wird, sodass der Dateipfad beliebig ist.

echo -e 'POST /command/udm/license/import HTTP/1.1\r\nHost: 10.200.XX.XX\r\nConnection: keep-alive\r\nX-Umc-Session-ID: XXXXr\nX-Forwarded-For: 192.168.0.XX\r\nContent-Length: 41\r\nUser-Agent: Mozilla/5.0\r\nContent-Type: application/json\r\n\r\n{"options": [{"tmpfile":"/etc/shadow"}]}' | nc -q 1 localhost 8090

Der Pfad sollte mindestens auf TEMPUPLOADDIR aus univention.management.console.protocol.session limitiert werden.
Comment 1 Florian Best univentionstaff 2012-08-13 09:49:00 CEST
Index: umc/python/udm/__init__.py
===================================================================
--- umc/python/udm/__init__.py  (Revision 34787)
+++ umc/python/udm/__init__.py  (Arbeitskopie)
@@ -46,6 +46,7 @@
 from univention.management.console.modules import Base, UMC_OptionTypeError, UMC_OptionMissing
 from univention.management.console.modules.decorators import simple_response
 from univention.management.console.log import MODULE
+from univention.management.console.protocol.session import TEMPUPLOADDIR
 
 import univention.admin.modules as udm_modules
 import univention.admin.objects as udm_objects
@@ -189,6 +190,8 @@
                if isinstance( request.options, ( list, tuple ) ):
                        # file upload
                        filename = request.options[ 0 ][ 'tmpfile' ]
+                       if not os.path.realpath(filename).startswith(TEMPUPLOADDIR):
+                               self.finished(request.id, [{'success': False, 'message': 'invalid file path'}])
                else:
                        self.required_options( request, 'license' )
                        lic = request.options[ 'license' ]
Comment 2 Florian Best univentionstaff 2012-08-14 14:40:31 CEST
Fixed:
univention-management-console-module-udm (2.0.5-1) unstable; urgency=low
  * added validation of upload path in licence import; Bug #28189
Comment 3 Alexander Kläser univentionstaff 2012-09-18 12:58:56 CEST
(In reply to comment #0)
> Der Lizenzupload erlaubt das Hochladen der Lizenz (über einen HTTP Upload) und
> ein normaler json-request mit dem Inhalt der Lizenz.
> 
> Am Ende vom Lizenz-upload wird die "temporäre Lizenz"-Datei gelöscht.
> 
> Im Code wird nicht ausreichend geprüft, welcher Typ von upload verwendet wird,
> sodass der Dateipfad beliebig ist.
> ...
> Der Pfad sollte mindestens auf TEMPUPLOADDIR aus
> univention.management.console.protocol.session limitiert werden.

Für UDM funktioniert das nun. Wie besprochen sollte diese Überprüfung generisch für alle Uploads durchgeführt werden, sonst stolpert man immer wieder über diese Lücke.

Changelog-Eintrag: "arbitary" sollte "arbitrary" heißen
Comment 4 Florian Best univentionstaff 2012-09-18 17:21:14 CEST
(In reply to comment #3)
> Für UDM funktioniert das nun. Wie besprochen sollte diese Überprüfung generisch
> für alle Uploads durchgeführt werden, sonst stolpert man immer wieder über
> diese Lücke.
udm reverted, Änderungen in umc-server und umc-web-server gemacht.
univention-management-console-module-udm (3.0.8-1)
univention-management-console (5.0.24-1)
univention-management-console-frontend (2.0.62-1)
 
> Changelog-Eintrag: "arbitary" sollte "arbitrary" heißen
Wurde angepasst.
Comment 5 Alexander Kläser univentionstaff 2012-09-18 17:52:04 CEST
Achtung, arguments ist jetzt immer gesetzt, d.h. das generische Upload-Kommando funktioniert jetzt nicht mehr:
====================
-               if path:
-                       self._log( 'info', 'Send upload to module function' )
-                       req = umcp.Request( 'COMMAND', arguments = [ path ] )
-               else:
-                       req = umcp.Request( 'UPLOAD' )
+               req = umcp.Request( 'UPLOAD', arguments = [ path ] )
====================
Besser:
  if path:
      req = umcp.Request( 'UPLOAD', arguments = [ path ] )
  else:
      req = umcp.Request( 'UPLOAD')

Ansonsten OK, trotzdem → REOPEN
Comment 6 Florian Best univentionstaff 2012-09-19 09:00:22 CEST
(In reply to comment #5)
> Achtung, arguments ist jetzt immer gesetzt, d.h. das generische Upload-Kommando> Besser:
>   if path:
>       req = umcp.Request( 'UPLOAD', arguments = [ path ] )
>   else:
>       req = umcp.Request( 'UPLOAD')
> 
> Ansonsten OK, trotzdem → REOPEN
Hm, ich habe die Überprüfung im umc-server gemacht, damit auch umc-upload weiterhin funktioniert:
Es wird geprüft, ob es argumente gibt und ob arguments[0] not in ('', '/').
univention-management-console 5.0.25-1

Der Bug bleibt trotzdem erstmal auf, weil:

(In reply to comment #3)
> Für UDM funktioniert das nun. Wie besprochen sollte diese Überprüfung generisch
> für alle Uploads durchgeführt werden, sonst stolpert man immer wieder über
> diese Lücke.
Es ist zwar jetzt generisch für alle uploads, aber udm/license/import bzw. alle anderen uploads sind nicht limitiert auf ein UMCP UPLOAD, es kann auch UMCP COMMAND benutzt werden und dadurch können wieder Dateien gelöscht werden.
Comment 7 Florian Best univentionstaff 2012-09-19 13:43:49 CEST
(In reply to comment #6)
> Der Bug bleibt trotzdem erstmal auf, weil:
> 
> (In reply to comment #3)
> > Für UDM funktioniert das nun. Wie besprochen sollte diese Überprüfung generisch
> > für alle Uploads durchgeführt werden, sonst stolpert man immer wieder über
> > diese Lücke.
> Es ist zwar jetzt generisch für alle uploads, aber udm/license/import bzw. alle
> anderen uploads sind nicht limitiert auf ein UMCP UPLOAD, es kann auch UMCP
> COMMAND benutzt werden und dadurch können wieder Dateien gelöscht werden.
Für UDM licence import wurde die ungenerische Prüfung wieder hinzugefügt.
Für das generelle upload Problem wurde Bug #28605 erstellt.

univention-management-console-module-udm (3.0.6-2)
Comment 8 Dirk Wiesenthal univentionstaff 2012-10-19 16:05:54 CEST
Es fehlt ein return nach der Fehlermeldung. Man kriegt zwar eine Meldung, dass das nicht erlaubt sei, aber die Funktion läuft danach weiter und hat meine /etc/shadow Datei gelöscht!

Außerdem ist die Meldung nicht übersetzt.

Darüber hinaus werden die Argumente nur unzureichend überprüft. Man kann mit Leichtigkeit TypeError und KeyError erzeugen. Das das ist nicht Teil des Bugs.

Changelog Ok
Comment 9 Florian Best univentionstaff 2012-10-22 09:48:50 CEST
(In reply to comment #8)
> Es fehlt ein return nach der Fehlermeldung. Man kriegt zwar eine Meldung, dass
> das nicht erlaubt sei, aber die Funktion läuft danach weiter und hat meine
> /etc/shadow Datei gelöscht!
> 
> Außerdem ist die Meldung nicht übersetzt.
> 
> Darüber hinaus werden die Argumente nur unzureichend überprüft. Man kann mit
> Leichtigkeit TypeError und KeyError erzeugen. Das das ist nicht Teil des Bugs.
> 
> Changelog Ok
Ja, das return fehlte natürlich!

Die Meldung braucht nicht übersetzt werden, da requests, die durch das frontend gehen diese Meldung nicht erhalten können.

Die anderen Fehler waren schon vorher da, sind aber auch nicht so tragisch (macht nichts kaputt und wird in der Regel nicht falsch aufgerufen).

FIXED in univention-management-console-module-udm 3.0.32-1
Comment 10 Dirk Wiesenthal univentionstaff 2012-10-22 11:08:08 CEST
Funktioniert!
Comment 11 Stefan Gohmann univentionstaff 2012-12-12 21:08:12 CET
UCS 3.1-0 has been released: 
 http://forum.univention.de/viewtopic.php?f=54&t=2125

If this error occurs again, please use "Clone This Bug".