Qualität von Extensions messbar machen

[matze]

Hallo.

Angeregt durch eine Diskussion und von den vielen Meldungen (z.b. bei Heise) dass die größte Schwachstelle eines Systems meist die von Usern produzierten Erweiterungen sind, wollte ich mir mal Gedanken machen:

Und zwar konkret:
Vielleicht schaffen wir es, einen Katalog / eine Checkliste zu erstellen mit Dingen, die eine gute bzw eine schlechte Erweiterung ausmachen.
Was sollte man in Typolight benutzen, was nicht benutzen.
Wie definiert sich guter Stil und wie schlechter Stil.

Vielleicht kann man, wenn ein guter Katalog herauskommt, diese Punkte auch mal automatisch abprüfen lassen? Das wäre auf jeden Fall etwas, das mir vorschwebt und auf das ich hinarbeiten möchte.

Euer Matze

-----------------------------

Hier gleich mal meine Vorschläge:

Durchfall-Kriterien:

• Verwedung von $_POST, $_GET, $_REQUEST und $_COOKIE
• Verwendung von eval();
• Verwendung von mysql_query() oder mysqli_query()

Nur mal so aus der Hüfte geschossen....

[FloydThreepwood]

Sehr interessant, ich hab länger nach so einer 'Best Practice' Liste gesucht. Zwar lässt sich lange über die Grenzen automatisierter Softwareverifikation streiten. Aber ein Leitfaden, der über das Entwicklerhandbuch hinaus geht ist ne tolle Idee.

• korrektes Ableiten - Welche Klasse wann
• Mail, Session, Input, Request Objekt(e) statt der PHP Pendants
• Widgets statt Modulen
• REGXP

MFG Jan

[do_while]

Eine solche Checkliste ist sicherlich nicht nur zur Beruhigung beim Einsatz der Extension wichtig, sondern auch als Final-Checkliste für den Entwickler selbst ganz brauchbar.

Was mir spontan einfällt ist das Vorhandensein von .htaccess-Dateien in den Extension-Verzeichnissen, die nicht über http: gelesen werden sollen, wo nur PHP mit den richtigen Rechten den Inhalt sehen darf.
Das variiert natürlich für jede Extension etwas, im Grunde sind es aber meist die Standard-Verzeichnisse:
- meinModul/config
- meinModul/dca
- meinModul/languages/*
- meinModul/templates

[Antipitch]

Hi,
einen Leitfaden würde ich auch begrüßen. Es muss vielleicht nicht so strikt und ausführlich sein wie bspw. hier beim Zend Framework, aber so ein paar Punkte wären evtl. nicht schlecht. Natürlich müssten die dann aber auch (selbst-)verpflichtend sein, denn sonst wäre es für die Katz, fürchte ich.

Spontan:
- Benennungen (Ordner, Klassen Funktionen)
- Code Formatierung
- (wie schon erwähnt) Kapselung der Session

Als Vorlage können natürlich Leos eigene Erweiterungen dienen...


cheers
Antipitch

[Toflar]

Was sich auch leicht prüfen lässt:

<?php if (!defined('TL_ROOT')) die('You can not access this file directly!');

[Psi]

An dieser Stelle wäre eine statische Quelltextprüfung toll. Leider habe ich nur mal kurz ein C# Tool in der Hand gehabt, aber sicher gibts hier auch diverse Pendants für PHP.

Diese könnten dann Fälle wie den Einsatz von mysql_query oder $_GET usw aufspüren und bemängeln. Natürlich müsste man sich hier einige Testfälle überlegen und auf Typolight abstimmen.

[lindesbs]

Meine Extension TranslationHelper macht in den Erweiterungen eine Analyse auf vorhandene Uebersetzungsfelder.

Es ist kein Problem eine Erweiterung zu schreiben, die durch alle Module crawlt und Auffaelligkeiten zeigt.

Fehlende htaccess, "verbotene" Woerter, missing "dies" usw.

Sollte fix erledigt sein.

[b2m]

Hi zusammen,

1. Klasse Idee!

2. was mir immer wieder auffällt ist, dass manche Leute absolut keine Ahnung von OOP haben. Dies äußert sich dadurch, dass zig Methoden neu definiert werden die im Framework eigentlich schon existieren. Noch schlimmer: die komplette Klasse im Framework wird ersetzt, ohne auf dessen Code zurückzugreifen. Solche Erweiterungen fallen bei mir sofort durch, weil das imho sehr schnell zu Inkompatibilität mit Typolight oder dem Framework kommt und zusätzliche Schwachstellen produziert.

Also imho falsches Vorgehen: ersetzen der kompletten Original-Klasse durch eigene Klasse um XY zu überprüfen.

Besseres Vorgehen: abgeleitete Klasse schreiben, Überprüfung XY vornehmen, dann an parent übergeben (wenn es nicht sogar mit einem Hook lösbar wäre...).

3. Mit einem Typolight können mehrere Projekte unter verschiedenen Domains betreut werden. Erweiterungen in den betreffenden Bereichen, die das auch 100% unterstützen gehören mit einer Extrabelobigung ausgezeichnet! <= "Auszeichnung" führt auf die falsche Fährte... meine eher eine Art "Kennzeichnung".

4. Zum Thema automatische Codevalidierung: da gab es doch eine ganze Artikelreihe auf phphatesme.com

Gruß b2m

[lindesbs]

So, ich habe eine Erweiterung soweit gerade fertig gemacht, die ein Modul freiwaehlbar kontrollieren kann

Hier im Wiki stehen die aktuellen Funktionen :

http://dev.typolight-forge.org/wiki/msc

Ich bitte Euch alle, mir mehr Input zu geben. Welche Suchwoerter muessen noch dazu, welche Kontrollen fehlen usw.

Das baue ich dann am WE alles noch mit dazu, und am Montag haben wir dann den ModuleSecurityChecker, entstanden aus den Wuenschen der Community,

Und da bezeichne jemand nochmal die Community als nicht hilfsbereit.....


BTW : Toflar & ich werden die Dinge dann im WIKI zusammentragen. Wer schon einen TL-Forge Account hat, und auch direkt im Wiki schreiben moechte, PN an mich, dann schalte ich ihn dafuer frei. Ansonsten Ideen hier im Thread oder als FeatuteWunsch in den Bugtracker.

Waer doch gelacht, wenn wir da nicht ein geiles Teilchen hinbekommen....

Screenshot :

[Antipitch]

Sehr geil, Stefan!

Vieleicht können wir so auch zu einer Art Klassifizierung/ Siegel ("Typolight Approved Extension" oder so) für's Repository gelangen. Vermutlich müssten aber auch individuelle/ visuelle Checks Niederschlag finden können (vgl. b2m's Posting). Es sollten dann aber gezielte Möglichkeiten des Feedbacks/ Autauschs vorhanden sein, zu einem "Extension Bashing" soll das ganze ja nicht führen...


cheers
Antipitch

[jan.theofel]

Hi,

ich möchte nur zu Bedenken geben, dass ein "Siegel" zugleich alle Module abwerten würde, die dieses Siegel nicht tragen. Dennoch sind diese vielleicht besserer bzw, sicherer implementiert weil sich dort ein Autor Gedanken um die saubere Nutzung scheinbar "böser" Funktionen gemacht hat statt sich blind auf einen automatisierten Test zu verlassen.

Dennoch halte ich einen solchen Schritt generell für begrüßenswert.

Jan

[Antipitch]

Hi Jan,

dass ein "Siegel" zugleich alle Module abwerten würde, die dieses Siegel nicht tragen.

genau, deshalb auch mein Hinweis mit dem Bashing.

Zunächst mal müsste abgeklärt werden, ob die Community und Leo eine erweiterte Qualitätskontrolle und Zertifizierung überhaupt möchten. Dann müsste es dafür klare Regularien und evtl. ein Team geben und die Zertifizierung sollte vermutlich eher beantragbar als Pflicht sein (wobei natürlich die Differenzierung von RCs/ Alphas zur Verfügung steht). Und im Repository müsste es dann eine genaue Erklärung geben, die unter anderem klarstellt, dass nicht zertifizierte Erweiterungen nicht per se schlecht sein müssen.

Ist ja auch erstmal nur ein Gedanke, der matzes Idee eines Leitfadens für die Entwicklerseite fortsetzt. Auch wenn die Feedback-Funktionen im Repository anscheinend nur spärlich genutzt werden: es scheint ja ausreichend Kommunikation zu geben und über das gros der Erweiterungen muss man sich wohl nicht beschweren...


cheers
Antipitch

[lindesbs]

Halt ! Ich mache das Modul NICHT um einer Extension eine Bewertung aufzudruecken. Es ist eher eine Unterstuetzung fuer den Entwickler, ob der sich an die Definitionen gehalten hat.
Also, ob ueberall die htaccess sind usw.

Es soll nicht dazu dienen, einen Index zur Bewertung zu machen.
Klar kann man schnell nachschauen, ob ein Modul "boese Woerter" hat oder sich nicht an Regelen haelt, aber mehr auch nicht.

[acenes]

Auch wenn die Feedback-Funktionen im Repository anscheinend nur spärlich genutzt werden

Richtig. Und die Möglichkeit zur Bewertung oder Rezension ist mit massiv weniger Aufwand verbunden als ein Code Review - wird trotzdem kaum genutzt.

Das ganze scheitert halt immer am Aufwand den es braucht um ein Gütesiegel zu validieren, und ohne Validierung duch kompetente Leute ist es nichts Wert. Abseits von bezahlten Inhalten ist so etwas leider kaum realistisch.

[matze]

So eine automatisierte Bewertung ist schön. Ich finds auch cool, dass du das jetzt schon gemacht hast.
Allerdings finde ich die Reihenfolge gerade falsch.

Wir sollte zuerst mal einen durchdachten Katalog/Liste aufstellen. Und wenn die fertig ist, dann kann man das umsetzen in Software.

Alles andere macht keinen Sinn. Der Entwickler sollte sich ja zuerst dieses Dokument durchlesen um dann seine Extension zu entwickeln. Erst nach der Entwicklung sollte dann noch der Test auf Fehler passieren um Flüchtigkeitsfehler zu verhindern.

Was ich in diesem Zusammenhang interessant fände:
In einem anderen Topic wurde gesagt, dass vor kurzem eine Extension aus dem Rep geflogen ist (irgendwas mit PayPal). Wäre mal interessant, was der entsprechende Autor falsch gemacht hat.

--------
EDIT: Das Problem mit der Bewertung im Rep liegt meiner Meinung nach nicht an der Faulheit der Leute sondern an der Art wie es kommuniziert wird. Ich Extensions z.B. nicht in meinem Backend bewerten sondern muss das auf der Website tun. Da bin ich aber nie. Außerdem ist die Bewertung nicht wirklich präsent kommuniziert.

[lindesbs]

Hm, Richtigstellung : Es geht keineswegs um eine "Bewertung"

Es ist einfach nur eine EntwicklerHilfe, so wie mein TranslationHelper (der mir fehlende Uebersetzungen anzeigt), mit der sich ein Entwickler vergewissern kann, das er alles "richtig" gemacht hat,

Es liegt mir fern ein automatisiertes BewertungsTool zu schreiben...

Das kann das menschliche Hirn einfach besser.

[hschottm]

Also ich finde es gut so einen Check zu haben, um schon mal grundlegende Inkonsistenzen mit den Vorgaben aufzudecken. Man muss ja auch sagen, dass diejenigen, die "verbotene" Elemente wie $_GET, $_POST, Umgehen der Datenbankabstraktion, htaccess so was nicht absichtlich machen, sondern eher aus Unwissenheit. Das Tool von lindesbs kann da schon mal ganz grobe Dinge aufdecken, die jeder beachten sollte und das finde ich sehr gut. Das sollte aber dann auch fernab von jeglicher automatischer Bewertung sein.

Ich weiß wie das ist und vielen ist es wahrscheinlich ähnlich gegangen: Man nimmt sich bestehende Module, schaut sich an, was die machen und versucht sein Modul ähnlich zu gestalten. Das sollte obige Fehler eigentlich schon mal vermeiden, aber offensichtlich ist es ja nicht so. Also Daumen hoch für eine solche Möglichkeit zur Überprüfung. Es wäre sogar cool wenn man so was im Repository eingebaut hätte und dann als Entwickler ein Warning angezeigt bekommt.

Das mit den Framework-Funktionen ist natürlich so ne Sache. Ich habe mich auch häufig dabei ertappt, dass mir irgendwann aufgefallen ist, dass ich Funktionen nachprogrammiert habe, die es eigentlich schon gibt. Nun dokumentiert Leo seine Arbeit super, daran kanns nicht liegen. Mittlerweile suche ich auch mal länger nach etwas vorhandenem, um nicht in die Verlegenheit zu geraten, es selbst noch mal zu schreiben. Hier sollte eigentlich (wenn man dann selbst zu faul, zu unorganisiert oder was auch immer ist) auch das Community-Forum helfen. Es ist ja in der Regel kein Problem mal ne Anfrage zu starten, ob's diese oder jene Funktionalität schon gibt. Da kriegt man in der Regel schnell 'ne Antwort. Bloß meistens läuft es in der Realität ja anders. Da will man nun auf Teufel komm raus schnell ein Modul fertig bekommen, weil man es irgendwo auch dringend braucht. Ich glaube, die meisten meiner Module sind mit der heißen Nadel gestrickt...

Lange Rede kurzer Sinn: Der oben angeregte Leitfaden: Von Entwicklern für Entwickler (und da möchte ich Leo auf jeden Fall außen vor halten, denn der hat schon genug Arbeit), wäre aus meiner Sicht eine tolle Sache. Ich würde auch gerne im bescheidenen Rahmen der mir verfügbaren Zeit an so etwas mitarbeiten, sollte sich da also eine Gruppe finden, ich bin dabei!

LG,
Helmut

[FloB]

Vor ein paar Tagen war im Forum auch schon die Rede von einem Beginner's Guide für TL-Programmierer. Kann man ja mit einem Leitfaden prima kombinieren!

[BugBuster]

Ich finde das Tool als reine Entwicklerhilfe sinnvoll, wie erwähnt macht man mal schnell was um es zum Laufen zu bekommen und vergisst dann später das eine oder andere.

Die Bewertungen im ER bringen mir so gut wie garnichts, besonders wenn dort beispielsweise bei Bedienung eine schlechte Bewertung erfolgt.
Erstes ist halt nicht alles möglich und zweitens kann nicht ich so nicht erfahren was denn so stört.

In den Rezensionen wiederum steht nicht dabei, für welche Version diese war.
(Nachtrag: Ups, steht ja doch da, hab ich übersehen)

Eine Bewertung macht doch nur Sinn, wenn ich die Auswahl habe an Alternativen.

Außerdem, das wiederum müsste dann in den Guide, was tue ich als Entwickler wenn mir das Tool sagt ich hätte den Error 47 begangen....

[Antipitch]

Hi Leute,

Ich mache das Modul NICHT um einer Extension eine Bewertung aufzudruecken.

so hatte ich das auch nicht verstanden bzw. war meine Antwort auch nicht gemeint. Sorry, falls das missverständlich war.

Wir sollten zuerst mal einen durchdachten Katalog/Liste aufstellen. Und wenn die fertig ist, dann kann man das umsetzen in Software.Alles andere macht keinen Sinn.

Ich denke Stefan wollte zunächst mal zeigen, dass sowas durchaus und relativ fix durch Software unterstützt werden kann. Bevor es zu irgenwelchen releases kommt, sollte eine sinnvolle Planung und Veröffentlichungsabfolge natürlich geklärt sein.

Abseits von bezahlten Inhalten ist so etwas leider kaum realistisch.

Muss ich dir natürlich leider Recht geben. Schon die notwendige Organisationsstruktur für die 2. Hälfte wirkt abschreckend. Aber wer weiß... Die erste Hälfte (Leitfaden/ Tool für Entwickler) wäre aber ja zumindest schon mal was.

Die Bewertungen im ER bringen mir so gut wie garnichts, besonders wenn dort beispielsweise bei Bedienung eine schlechte Bewertung erfolgt.

Das stimmt wohl, ich denke aber eh, dass Dinge wie Bedienungsfreundlichkeit eher im normalen Feedback verbleiben sollten. Und um die Feedbackmöglichkeiten zu pushen (vgl. auch acenes posting), könnte da ja ruhig mal eine Einleitung/ Teaser oben im Rep stehen (mit Verlinkung auf eine längere Erklärung bzgl. Sinn u. Möglichkeiten).

Error 47? Ist glaub ich "Zu früh ins Bett gegangen", oder ?


cheers
Antipitch

PS für die Liste:
1. Nicht erweiterbare blanke Helperfunktionen im Modul
2. Bequemlichkeits-Vollballern des Config-Arrays (auch wenn auf selbiges ab 2.8 händisch mehr Einfluss genommmen werden kann).

[lindesbs]

Antipitch hats richtig gedeutet. Es geht erstmal darum, "was ist moeglich". Die Basis fuer die Auffindung von "Problemstellen" ist gegeben, was nun gefunden werden soll, muss halt die Community definieren.

[matze]

Genau das wollte ich ja von Anfang an mit diesem Topic erreichen.
Also fangen wir mal frisch an

Gibt es denn vielleicht Funktionen in PHP die von PHP aus nicht sicher sind, weil sie manche Zeichensätze nicht wirklich unterstützen? Ich dachte ich hab da mal was gelesen...

[Toflar]

Ich hab mal das Wiki mit Config-Daten erweitert, die man eigentlich nicht braucht. Ist sicher nicht abschliessend

[b2m]

Hi zusammen,

ich habe hier mal noch eine Liste von Funktionen, die imho nicht verwendet werden sollten (wenn sie nicht sogar schon Serverseitig gesperrt sind):

• apache_get_modules()
• apache_get_version()
• apache_getenv()
• apache_setenv()
• dir()
• dl()
• eval()
• exec()
• fsockopen()
• get_defined_functions()
• highlight_file()
• ini_alter()
• ini_restore()
• ini_set()
• passthru
• phpinfo()
• show_sources()
• symlink()
• system()
• tmpfile()

btw: könntet ihr die gesperrten Funktionen im Wiki auch alphabetisch ordnen?

Des Weiteren könnte man sich die in Typolight verwendeten Design-Patterns mal noch genauer ansehen. Häufig benötigte Klassen wie Database selbst zu organisieren ist generell unnötig und zeugt von Unkentniss der API.

Ich wollte außerdem noch einmal anregen sich mit dem CodeSniffer zu beschäftigen (siehe auch Artikelreihe auf phphatesme. Damit könnten wir sehr viel tiefer gehende Coding-Standards festlegen und auch Anfängern eine Hilfe bezüglich Codeeinrückung, Kommentare... geben.

Gruß b2m

[b2m]

Hi zusammen,

habe ich total vergessen: was ist mit $_FILES und $_SESSION?
Sowie den veralteten Version $HTTP_SESSION_VARS, $HTTP_GET_VARS usw.?

Gruß b2m

[Toflar]

Wiki angepasst. Auch alphabetisch jetzt.

[Sebastian]

HI

mal eine Frage: Prüft das Modul nur in system/modules/*, oder kann es auch Dateien an anderen Orten von TL testen? Oder haltet ihr das für irrelevant?

Sebastian

[lindesbs]

Es sucht dort, wo ICH will, da ich aber davon ausgehe, das der CORE an sich "Safe" ist, wird nur system/modules kontrolliert. Dort alle Dateien, deren Extension vorgegeben ist, also "php,tpl,inc".
Man koennte noch template hinzunehmen, das veraendert sich ja nicht ohne Wissen des Anwenders.

[matze]

hmmm...
Wie wäre es denn, wenn man das Schlüsselwort new als böse deklariert.
Denn eigentlich soll man ja keine Instanzen direkt erstellen sondern via import() die Klassen nutzen.
Evtl müsste man dann das new zusammen mit den bei TL eingebauten Klassen ergänzen...

[BugBuster]

hmmm...
Wie wäre es denn, wenn man das Schlüsselwort new als böse deklariert.
Evtl müsste man dann das new zusammen mit den bei TL eingebauten Klassen ergänzen...

Ich benutze new, allerdings nicht um TL internes zu nutzen sondern um eigene zusätzliche Klassen oder Klassen von dritte die innerhalb meines Modules liegen zu nutzen. Daher wäre rein nach new zu suchen nicht so günstig.

[Sebastian]

HI

ich dachte auch daran, dass manche Module etwas in plugins/ oder in system/drivers ablegen.

Sebastian

[do_while]

Wie wäre es denn, wenn man das Schlüsselwort new als böse deklariert.

Was ist aber, wenn ich eine Datei erstellen möchte?
Dann brauche ich eine Instanz von File, also new File( $dateiname ).
Wenn ich das anders mache arbeite ich wieder am Framework vorbei - ein Teufelskreis!

Weitere Beispiele:
$objTemplate = new BackendTemplate('be_wildcard');
$this->Date = new Date(date('Ymd'), 'Ymd');

Ich denke, das mit dem new verbieten, sollte man auslassen.

[FloB]

• symlink()

Was macht dat denn hier? Dafür gibts keine Wrapper-Funktion in TL, also sollte das auch nicht auf der Liste auftauchen. Ich sehe auch nicht, wo diese Funktion unsicher sein soll (bis auf falsche Benutzung)?

[matze]

deswegen meinte ich ja weiter oben: New nur mit Einschränkung auf bestimmte Klassen.

Also new Database ist nicht der Hit, da man dann besser import('Database') verwenden soll.

Oder?

[BugBuster]

jo, so passt das.

[Toflar]

Hey Leute

Mal ehrlich: So langsam driften wir vom Thema ab. Ich denke wir sollten uns nicht Gedanken zum Coding-Stil machen und auch nicht darüber, ob TL-eigene Methoden und Klassen verwendet werden oder nicht.

Hier geht es um die Sicherheit. Mir ist doch egal, ob jemand eigene header schreibt anstatt sendFileToBrowser() nutzt. Mir ist auch egal, ob jemand eine eigene redirect-Methode schreibt oder $this->redirect() nutzt.
Solange die Sache sauber und sicher geschrieben ist, kann derjenige von mir aus alles selber schreiben.

Nicht, dass ich gegen Code-Reusage wäre - im Gegenteil! Aber ich denke, schlussendlich lässt sich der Coding-Stil eh nicht kontrollieren bzw. ich finde darüber sollten wir auch nicht urteilen.

Es geht hier vor allem um die Sicherheit der Extensions und somit TL an sich. Deshalb ist der jetzige BadWord-Approach schon der richtige Weg

[b2m]

Hi
@FloB: symlink() ist sehr hilfreich dabei bei hacking-Attacken aus dem Webroot herauszuspringen (natürlich abhängig von Severkonfig blablabla...)

- - - - - -

Bezüglich Ort: wenn wir schon von Sicherheit reden - warum nicht die ganze Installation überprüfen? Dann nimmt andersweitig hochgeladenen schädlichen Code gleich noch mit und kann sich gegenebenfalls Gedanken darüber machen wo denn das herkommt.

- - - - - -

@Toflar: Coding Stil: lässt sich problemlos kontrollieren (siehe z.B. PEAR). Ein für Typolight erweiterter Regelsatz würde es gerade auch Anfängern erleichtern. Aber ich vermute mal, dass sich kein Entwickler gerne in seiner Kreativität einschränken lassen will.

Gruß b2m
PS: ich dachte es ginge um Qualität und um Sicherheit, aber wir können das Thema natürlich auch auf den wichtigeren Aspekt der Sicherheit reduzieren.

[FloB]

@FloB: symlink() ist sehr hilfreich dabei bei hacking-Attacken aus dem Webroot herauszuspringen (natürlich abhängig von Severkonfig blablabla...)

Mit vielen anderen Funktionen schafft man das auch . Es gibt aber wie gesagt keine Wrapper-Funktion im Framework, somit würde diese Info angekreidet werden, wenn jemand eine Erweiterung bastelt, die Symlinks erzeugt.

Bezüglich Ort: wenn wir schon von Sicherheit reden - warum nicht die ganze Installation überprüfen? Dann nimmt andersweitig hochgeladenen schädlichen Code gleich noch mit und kann sich gegenebenfalls Gedanken darüber machen wo denn das herkommt.

Der Core nutzt einige der Badwords – und nicht nur die Dateien in /system/libraries/. Das spricht dagegen. Und nein, ausschließen der bestimmten Ordner ist nicht effektiv – eine Erweiterung kann theoretisch in jeden Ordner Dateien ablegen. Ein Ausschließen von Dateilisten bringt ebenso wenig – zu viele Daten und Core-Dateien können auch überschrieben werden.

Dennoch: Es sollte noch überprüft werden, ob Extensions versuchen Dateien in einem Core-Extension-Ordner abzulegen oder anderen Ordnern, die nicht dafür gedacht sind (Temp-, Log-Verzeichnis etc.).

[deerwood]

Moin Leo, alle,

... Eine Extensionüberprüfung wie ihr das hier vorschlagt ist definitiv der falsche Weg. Es ist einfach nicht möglich Programmcode maschinell auf Sicherheit prüfen zu lassen. So einen Code Audit muss IMMER ein Programmierer selbst machen, alles andere bringt es einfach nicht. ...

Ich verstehe die Erweiterung von lindesbs als SEHR hilfreiches Tool, wie etwa lint. Es weist schlicht auf Stellen im Code hin, die fragwürdig sind. Aber es verhindert und bewertet nichts. Sehr hilfreich für Neulinge und gestresste Programmierer.

Wenn der Check NICHTS meldet, dann ist sicher noch nicht alles OK, aber viele grobe Fehler sind vermieden (ich hatte mal jemanden, der direkt $_GET Variablen ungeprüft in einen SQL-Query-String eingebaut hatte, auf die Probleme hingewiesen ... der war dann tödlich beleidigt )

WENN der Check Warnungen ausspuckt, dann heisst das ja noch lange nicht, dass der Code schlecht ist ... falls, wie in Deinem Fall z.B., jemand weiss, was er tut. Dennoch ist das oft ein Hiweis darauf, dass man etwas verbessern kann, etwa besser kapseln oder Helfer-Klassen zur Kollaboration einführen oder ein Feature-Ticket schreiben usw.

Der Programmier-Stil sollte sicher nicht geprüft werden und das hat lindesbs, soweit ich sehen kann, auch nicht vor. (Ich persönlich habe z.B. riesige Probleme, komplexe einzilige SQL Statements zu verstehen, egal, wie breit mein Bildschirm ist. Etwa mit JOINS und SUBQUERIES).

Meine Gedanken zur Erweiterung:

• Kategorisierung/Tagging der Prüfergebnisse, z.B. Sicherheitsrelevant, Fehler, Warnung, Hinweis
• Hilfetexte bzw. Hinweise, wie man es besser machen kann. Z.B. statt $_GET $this-Input-get() verwenden.

Ich werde das Tool nutzen ... sei es auch nur, um auf fehlende .htaccess Datein hinzuweisen (hoffentlich).

LG, Georg

[b2m]

Hi zusammen,

bzgl. symlink(): es war ein Vorschlag (umso besser, dass darüber diskutiert wird). Was ich eben meinte ist, dass Schadcode diese Funktion beinhalten wird und die Module, die diese Funktion enthalten sich dafür in Grenzen halten werden. Deshalb kann imho durchaus eine getaggte/kategorisierte Warnung wie von deerwood vorgeschlagen a la: symlink in Verwendung blubb ausgegeben werden, was dann manuell nachgeprüft werden kann.

@leo.unglaub: Bezüglich new - doch du kannst Mails ohne new verschicken (z.B. mit dem zweiten, optionalen Parameter von System::import()), nur über den Sinn lässt sich streiten. Außerdem ging es eher darum bestimmte Core Klassen wie die Datenbank nicht ständig selbst zu organisieren, sondern das der Klasse von der man ableitet zu überlassen.

Auch hier werden sich die Fälle in denen das nötig ist sich so in Grenzen halten, dass dies manuell geprüft werden kann.

Ich persönlich verstehe das Tool als Hilfestellung und nicht als ultimo ultra... Es weist mich auf Stellen hin, die ich mir noch einmal genauer ansehe und mit sinnvollen Meldungen ist es durchaus auch für Anfänger eine Hilfe.

@FloB: natürlich nutzt der Core einige Badwords... irgendwo muss ja mal angefangen werden Aber auch das sollte einem auch nur annähernd erfahrenen Entwickler keine Probleme bereiten.

Gruß b2m