Abo
  1. Foren
  2. Kommentare
  3. Security
  4. Alle Kommentare zum Artikel
  5. › Bundeshack: Hack auf Bundesregierung…

Ein Blick in die DB Klasse von Ilias spricht für sich

  1. Thema
  1. 1
  2. 2

Neues Thema Ansicht wechseln


  1. Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: superkek 08.03.18 - 16:22

    Was soll man zu sowas noch sagen:

    https://github.com/ILIAS-eLearning/ILIAS/blob/6f3e56a4357889c525ef0d4a4814aaa1c9f89279/Services/Database/classes/PDO/class.ilDBPdo.php

  2. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: fabian2k 08.03.18 - 16:45

    Das sieht so aus als ob zumindest in dieser Klasse die Parameter direkt in den SQL String eingebaut werden, ohne jeden Versuch es irgendwie abzusichern. Ich vermute mal das an anderen Stellen versucht wird die Parameter zu filtern, aber sowas ist sehr fragil und kann ganz leicht zu Sicherheitslücken führen.

    Es gibt wirklich keinen Grund nicht grundsätzlich parametrisierte Queries zu verwenden. Es ist ja noch nicht mal schwieriger oder aufwändiger als dieser Murks.

  3. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: trolling3r 08.03.18 - 16:53

    Aha und was genau ist das Problem?

  4. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Hirngespinste 08.03.18 - 16:56

    Wenn ich mir allgemein den Code so anschaue, rollen sich mir die Fussnaegel auf.
    Anstatt Eingaben zentral in der DB-Abstraktion zu maskieren, lieber natives SQL zulassen und darauf hoffen, dass der restliche Code immer brav ilDBPdo::quote() verwendet.

    Anscheinend wissen sie auch nicht was der Unterschied zw. Single u. Double Quotes in PHP ist, sonst wuerden sie es nicht wild mischen; inkl. ab und zu getrenntem konkatenieren von Double Quotes und Variablen, und dann an anderer Stelle Variablen innerhalb Double Quotes.
    Absolut kein einheitlicher Code-Style, manchmal K&R-, manchmal Allman-Klammersetzung.
    Wuerde es sowas bei mir im Team geben, gaeb's erstmal 'nen Einlauf.

    Richtige Profis, topkek.

  5. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Genie 08.03.18 - 17:21

    Hirngespinste schrieb:
    --------------------------------------------------------------------------------
    > Anscheinend wissen sie auch nicht was der Unterschied zw. Single u. Double
    > Quotes in PHP ist, sonst wuerden sie es nicht wild mischen; inkl. ab und zu
    > getrenntem konkatenieren von Double Quotes und Variablen, und dann an
    > anderer Stelle Variablen innerhalb Double Quotes.
    > Absolut kein einheitlicher Code-Style, manchmal K&R-, manchmal
    > Allman-Klammersetzung.
    > Wuerde es sowas bei mir im Team geben, gaeb's erstmal 'nen Einlauf.

    Nix gegen saubere Schreibweisen, aber wenn du sonst keine Sorgen hast... ;-)

  6. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: nikeee13 08.03.18 - 20:01

    Wisst ihr was? Es ist Open source! IHR könnt es fixen!

  7. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Tigtor 08.03.18 - 21:23

    Genie schrieb:
    --------------------------------------------------------------------------------
    > Hirngespinste schrieb:
    > ---------------------------------------------------------------------------
    > -----
    > > Anscheinend wissen sie auch nicht was der Unterschied zw. Single u.
    > Double
    > > Quotes in PHP ist, sonst wuerden sie es nicht wild mischen; inkl. ab und
    > zu
    > > getrenntem konkatenieren von Double Quotes und Variablen, und dann an
    > > anderer Stelle Variablen innerhalb Double Quotes.
    > > Absolut kein einheitlicher Code-Style, manchmal K&R-, manchmal
    > > Allman-Klammersetzung.
    > > Wuerde es sowas bei mir im Team geben, gaeb's erstmal 'nen Einlauf.
    >
    > Nix gegen saubere Schreibweisen, aber wenn du sonst keine Sorgen hast...
    > ;-)

    Naja, es macht halt schon auch Sinn, wenn man sich im vorrais einigt nach welchem system variablen, Klassen etc benannt werden usw. Insbesondere im Team. Wenn man dann noch quotes und doublequotes ohne Sinn und verstand einsetzt ... Insbesondere bei PHP macht das ja doch schon auch einen unterschied.

    1000-7

  8. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: lastavasta 08.03.18 - 21:54

    Ich verwahre mir jegliche Kritik an dieser Stelle!

    Dies ist eine Open Source Entwicklung, der Source Code kann von jedem frei eingesehen und geprüft werden - er ist somit sicher.

    </Irony_end>

  9. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: burzum 08.03.18 - 22:48

    nikeee13 schrieb:
    --------------------------------------------------------------------------------
    > Wisst ihr was? Es ist Open source! IHR könnt es fixen!

    Bezahl mich dafür diesen Murks in Form zu bringen und ich machs.

    Das Projekt ist nur ein weiteres Beispiel (und leider gibt es gerade sehr viele in php) für ein Projekt das sicher eher populär ist, weil es gratis ist und sicher nicht wegen überragender Code Qualität und Sicherheit...

    lastavasta schrieb:
    --------------------------------------------------------------------------------
    > Ich verwahre mir jegliche Kritik an dieser Stelle!
    >
    > Dies ist eine Open Source Entwicklung, der Source Code kann von jedem frei
    > eingesehen und geprüft werden - er ist somit sicher.

    So wie bei Hearthbleed oder zahlreichen Exploits im Linux Kernel oder Anwendungen des Linuxsystems? Das ist ALLES GANZ GANZ SICHER! Weil MEINE, JA!, MEINE!!! Oma hat den Code bei einer Tasse Kaffee reviewed und als GANZ SICHER eingestuft!

    ;-p

    Ich halte Open Source für kein Stück sicherer als Closed Source, nur wird ein Bug in der Regel schneller gefixt - WENN ihn denn mal einer gefunden hat.

    Ash nazg durbatulûk, ash nazg gimbatul, ash nazg thrakatulûk agh burzum-ishi krimpatul.

  10. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Tragen 08.03.18 - 23:05

    fabian2k schrieb:
    --------------------------------------------------------------------------------
    > Es gibt wirklich keinen Grund nicht grundsätzlich parametrisierte Queries
    > zu verwenden. Es ist ja noch nicht mal schwieriger oder aufwändiger als
    > dieser Murks.

    Leider doch. Das Debuggen ist mega ätzend kompliziert da man den
    "fertigen" SQL Befehl gar nicht mehr zu Gesicht bekommt.

    Wir sind davon wieder weg und haben andere Lösungen die auch
    nicht zu SQL Injections führen können.

    Tragen

  11. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: emuuu 08.03.18 - 23:34

    Tragen schrieb:
    --------------------------------------------------------------------------------
    > fabian2k schrieb:
    > ---------------------------------------------------------------------------
    > -----
    > > Es gibt wirklich keinen Grund nicht grundsätzlich parametrisierte
    > Queries
    > > zu verwenden. Es ist ja noch nicht mal schwieriger oder aufwändiger als
    > > dieser Murks.
    >
    > Leider doch. Das Debuggen ist mega ätzend kompliziert da man den
    > "fertigen" SQL Befehl gar nicht mehr zu Gesicht bekommt.
    >
    > Wir sind davon wieder weg und haben andere Lösungen die auch
    > nicht zu SQL Injections führen können.
    >
    > Tragen

    Das ist wie zu sagen, „anschnallen beim Autofahren ist mega ätzend“. Sql ohne Parameter ist einfach fahrlässig.
    Außerdem noch plain SQL verwenden könnte man auch vermeiden.. es gibt für wirklich jede Sprache zig ORM die einem das zusätzlich abnehmen/vereinfachen.

    Bei .net muss ich hier mal ne Lanze für Dapper brechen :)

  12. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: burzum 08.03.18 - 23:52

    Tragen schrieb:
    --------------------------------------------------------------------------------
    > fabian2k schrieb:
    > ---------------------------------------------------------------------------
    > -----
    > > Es gibt wirklich keinen Grund nicht grundsätzlich parametrisierte
    > Queries
    > > zu verwenden. Es ist ja noch nicht mal schwieriger oder aufwändiger als
    > > dieser Murks.
    >
    > Leider doch. Das Debuggen ist mega ätzend kompliziert da man den
    > "fertigen" SQL Befehl gar nicht mehr zu Gesicht bekommt.

    Sicher das das nicht geht? Ich müßte mal in unserem Code graben aber das Debug Tool zeigt mir die Queries an. Ich habe einen Logger geschrieben der nur lahme Queries von über X ms in das Log der Anwendung schreibt und die landen als pures SQL darinnen.

    Und selbst wenn nicht, einfach ein tail auf das Query Log des SQL Servers ansetzen? Im Livebetrieb geht das natürlich nicht.

    Ash nazg durbatulûk, ash nazg gimbatul, ash nazg thrakatulûk agh burzum-ishi krimpatul.



    1 mal bearbeitet, zuletzt am 08.03.18 23:53 durch burzum.

  13. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Tragen 09.03.18 - 01:40

    emuuu schrieb:
    --------------------------------------------------------------------------------

    > Das ist wie zu sagen, „anschnallen beim Autofahren ist mega
    > ätzend“. Sql ohne Parameter ist einfach fahrlässig.
    > Außerdem noch plain SQL verwenden könnte man auch vermeiden.. es gibt für
    > wirklich jede Sprache zig ORM die einem das zusätzlich
    > abnehmen/vereinfachen.

    Nicht wirklich. Du benutzt einen Sicherheitsgurt wo du nicht siehst
    wie er aussieht und wenn er mal nicht funktioniert weißt du kaum
    warum.
    Bei unserer Lösung kann auch kein SQL Injection auftreten aber du bist
    viel freier beim arbeiten mit SQL.

    > Bei .net muss ich hier mal ne Lanze für Dapper brechen :)

    ORM Mapper sind das schlimmste.
    Ich sehe keinen einzigen Vorteil außer für Programme die ich
    vielleicht in 2 Monaten runterschreiben muss und dann nie
    wieder brauche. Software die du 10 bis 20 Jahre warten musst
    damit zu programmieren ist der größte Fehler überhaupt.

    Tragen

  14. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Tragen 09.03.18 - 01:42

    burzum schrieb:
    --------------------------------------------------------------------------------

    > Sicher das das nicht geht? Ich müßte mal in unserem Code graben aber das
    > Debug Tool zeigt mir die Queries an. Ich habe einen Logger geschrieben der
    > nur lahme Queries von über X ms in das Log der Anwendung schreibt und die
    > landen als pures SQL darinnen.

    Nein das geht nicht und kann nicht gehen. Der Datenbankserver bekommt
    auch nur den Grundbefehl und die Parameter und erst der baut den
    SQL Befehl zusammen. Natürlich kann man "versuchen" das gleiche wie
    der Datenbankserver zu machen und den Befehl zusammen zu bauen aber
    es kommt manchmal nicht das gleiche raus.

    > Und selbst wenn nicht, einfach ein tail auf das Query Log des SQL Servers
    > ansetzen? Im Livebetrieb geht das natürlich nicht.

    Es gibt ja nicht nur SQL Server und es macht alles unnötig kompliziert.

    Tragen

  15. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: MoonShade 09.03.18 - 01:50

    burzum schrieb:
    --------------------------------------------------------------------------------
    > Ich halte Open Source für kein Stück sicherer als Closed Source

    Kommt ganz drauf an. Bei nem Projekt was sehr aktiv ist und wo man sieht dass viele Leute regelmäßig mitmischen, sehe ich die Chance eher als bei einem Unternehmen mit der Devise "mach mal, passt schon, funktioniert doch", wo Bugreports vermutlich mit "wurschtel da mal eben was zurecht um diese Nörgler vom CCC ruhig zu stellen" kommentiert werden.

    Gab ja schon genug solcher Fälle. OS ist nicht generell sicherer als CS nur weil es OS ist, aber ich glaube das behauptet auch keiner. Bei OS besteht zumindest die Chance einzuschätzen, zumindest Indikatoren zu haben, wie es wohl um die Sicherheit bestellt ist (wie werden mit Sicherheitslücken umgegangen, öffentliche Diskussionen auf Bugtrackern, etc.)

  16. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Baron Münchhausen. 09.03.18 - 09:03

    11 ist das Problem natürlich.

  17. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Baron Münchhausen. 09.03.18 - 09:18

    Meine Güte. Nutzt einfach bind parameter. Wo ist der Wurm... "unsere Lösung"... Ganz ehrlich, das ist doch keine Wissenschaft.

    Und zum Thema ORM. Wer einfach nur "X ist doof" bashing betreibt, versteht die Techniken nicht so richtig und will sich nicht damit auseinandersetzen oder klare Linie ziehen, wofür etwas gut ist oder nicht. Dann wird es falsch benutzt oder an einer Stelle an der es zu sogen. accidental comlexity führt und statt nachzudenken, wo der Wurm ist, wird einfach die Technik/Methode/Pattern gebasht.

    Dass ich mit ORM ganze Datengraphen aus eigentlich dafür nicht sehr gut geeigneten SQL Datenbank via graphql ziehen kann und das mit minimal joins und evtl. 5-10 Abfragen (Stichwort batch-query, dataloader), zeigt, dass man einfach die richtigen Ideen und Techniken benötigt. Und natürlich gibt es Situationen wo ich kein ORM nutzen würde.

    Das ist wie sich aufzuregen, dass man mit einem Teelöffer mühseelig die Suppe auslöffeln muss. Was für dumme Teelöffel. Die braucht doch kein Mensch, so umständlich. Würdet ihr herumgenörgle diesbezüglich nicht auch lächerlich finden? Aber wenn man das nicht so erkennt, dann ist ja alles im Butter fürs eigene SWG/Ego.



    4 mal bearbeitet, zuletzt am 09.03.18 09:21 durch Baron Münchhausen..

  18. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: fuzzy 09.03.18 - 09:22

    Uneinheitlicher Codestyle führt dazu, dass Fehler eher übersehen werden. Weil der andere Codestyle schon eine Abweichung ist.

    Wenn irgendwelche Mitwirkenden meinen, sie könnten den gemeinschaftlich vereinbarten Codestyle ignorieren, müssen sie aus dem Projekt entfernt werden. Eine positive und produktive Zusammenarbeit ist so nicht möglich, nicht nur in Sachen Codestyle.

    Baumansicht oder Zitieren oder nicht Posten - die Wahl ist eure!

  19. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Kenterfie 09.03.18 - 09:51

    Muss leider zustimmen. ORM ist leider mehr Problem als Lösung. Viele verlassen sich darauf und wissen oft nicht mehr was im Hintergrund passiert. Habe es oft genug erlebt, dass Abfragen um Faktor 1000 langsamer waren, weil der Mapper einen unnötig umständlicheren Weg genutzt hat, wo ein simpler plain sql Query gereicht hätte. Abstraktion vereinfacht die Entwicklung, aber wenn es um Performance geht macht man es lieber direkt.

  20. Re: Ein Blick in die DB Klasse von Ilias spricht für sich

    Autor: Sammie 09.03.18 - 10:01

    burzum schrieb:
    --------------------------------------------------------------------------------
    > Ich halte Open Source für kein Stück sicherer als Closed Source, nur wird
    > ein Bug in der Regel schneller gefixt - WENN ihn denn mal einer gefunden
    > hat.

    Aber Open Source bringt dafür das Problem, dass es potentiellen Angreifern deutlich einfacher gemacht wird, Schwachstellen im System zu finden. Ein sehr großer Teil der Angriffe auf Webserver geschieht ja genau auf diesem Wege. Man sieht, dass der Betreiber eine bestimmte Software laufen hat, guckt im Source noch potentiellen Angriffsflächen oder sucht sich fertige Lösungen aus Exploit-Datenbanken und ist im Handumdrehen im System.

    Da nützt auch der beste und schnellste Patch nichts, wenn die Software auf dem Webserver nie aktualisiert wird - und die Updatemoral ist nunmal bei den meisten eher gering - da gilt eher noch das "never change a running system"-Prinzip. Bloß nichts anfassen... es könnte ja was kaputt gehen. Und das zu fixen ist dann wieder teuer. Also lässt mans lieber sein.

    Würde jemand genau die gleichen Bugs in eine Eigenentwicklung bauen, deren Source nicht bekannt ist, müssten Angreifer dagegen relativ blind rumprobieren - und in der Regel ist der Aufwand viel zu hoch. Angreifer wollen es schnell und einfach, ohne großes Aufsehen, ohne 1000 Einträge diverser Hackversuche im Logfile, sonst ist es schwer unerkannt auf dem Zielsystem zu agieren. Closed Source mag nicht sicherer sein, was die Codequalität betrifft - aber dennoch bietet es durch den unbekannten Code weniger offensichtliche Angriffsfläche.

  1. 1
  2. 2

Neues Thema Ansicht wechseln


Um zu kommentieren, loggen Sie sich bitte ein oder registrieren Sie sich. Zum Login

Stellenmarkt
  1. Deloitte, verschiedene Einsatzorte
  2. CSP GmbH und Co. KG, Deutschland
  3. EWE Aktiengesellschaft, Bremen
  4. FC Basel 1893 AG, Basel (Schweiz)

Golem pur
  • Golem.de ohne Werbung nutzen

Anzeige
Hardware-Angebote
  1. ab 119,98€ (Release 04.10.)
  2. (reduzierte Überstände, Restposten & Co.)


Haben wir etwas übersehen?

E-Mail an news@golem.de


Lenovo Thinkpad T480s im Test: Das trotzdem beste Business-Notebook
Lenovo Thinkpad T480s im Test
Das trotzdem beste Business-Notebook

Mit dem Thinkpad T480s verkauft Lenovo ein exzellentes 14-Zoll-Business-Notebook. Anschlüsse und Eingabegeräte überzeugen uns - leider ist aber die CPU konservativ eingestellt und ein gutes Display kostet extra.
Ein Test von Marc Sauter und Sebastian Grüner

  1. Thinkpad E480/E485 im Test AMD gegen Intel in Lenovos 14-Zoll-Notebook
  2. Lenovo Das Thinkpad P1 ist das X1 Carbon als Workstation
  3. Thinkpad Ultra Docking Station im Test Das USB-Typ-C-Dock mit robuster Mechanik

iPhone Xs, Xs Max und Xr: Wer unterstützt die eSIM in den neuen iPhones?
iPhone Xs, Xs Max und Xr
Wer unterstützt die eSIM in den neuen iPhones?

Apples neue iPhones haben neben dem Nano-SIM-Slot eine eingebaute eSIM, womit der Konzern erstmals eine Dual-SIM-Lösung in seinen Smartphones realisiert. Die Auswahl an Netzanbietern, die eSIMs unterstützen, ist in Deutschland, Österreich und der Schweiz aber eingeschränkt - ein Ãœberblick.
Von Tobias Költzsch

  1. Apple Das iPhone Xr macht's billiger und bunter
  2. Apple iPhones sollen Stiftunterstützung erhalten
  3. XMM 7560 Intel startet Serienfertigung für iPhone-Modem

SpaceX: Milliardär will Künstler mit zum Mond nehmen
SpaceX
Milliardär will Künstler mit zum Mond nehmen

Ein japanischer Milliardär ist der mysteriöse erste Kunde von SpaceX, der um den Mond fliegen will. Er will eine Gruppe von Künstlern zu dem Flug einladen. Die Pläne für das Raumschiff stehen kurz vor der Fertigstellung.
Von Frank Wunderlich-Pfeiffer

  1. Mondwettbewerb Niemand gewinnt den Google Lunar X-Prize

  1. Amazon: Echo Show mit Browser, Skype und großem Display
    Amazon
    Echo Show mit Browser, Skype und großem Display

    Amazon hat die zweite Generation des Echo Show vorgestellt. Das neue Modell erhält ein deutlich größeres Display, soll besser klingen, kann auch mit Skype verwendet werden und bekommt gleich ein Smart-Home-Hub dazu. Am Preis ändert sich nichts.

  2. Smart Plug: Amazon bringt eigene smarte Steckdose auf den Markt
    Smart Plug
    Amazon bringt eigene smarte Steckdose auf den Markt

    Amazon hat seine erste eigene smarte Steckdose vorgestellt. Die Steckdose benötigt keinen Hub und kann direkt mit Alexa-Lautsprechern gesteuert werden.

  3. Echo Plus und Echo Dot: Zwei neue Alexa-Lautsprecher von Amazon
    Echo Plus und Echo Dot
    Zwei neue Alexa-Lautsprecher von Amazon

    Amazon hat zwei neue Echo-Lautsprecher vorgestellt. Der Echo Dot bleibt Amazons preiswerter Einstieg in die Welt smarter Lautsprecher, der neue Echo Plus ist für Käufer gedacht, die einen besseren Klang wünschen.


  1. 22:26

  2. 21:22

  3. 21:16

  4. 20:12

  5. 20:09

  6. 19:11

  7. 18:50

  8. 18:06