n'abend,

die grundlegende Basis für mein winziges (eigenes) CMS hab ich nun soweit fertig. (falls ich nich noch welche Fehler finde).

Und da wollte ich mal fragen, ob jemand hier zufällig irgendwo ein Sicherheitsproblem entdecken kann?
Jetzt kann ich ja Vieles noch problemlos grundlegend umstrukturieren.


als Zielplattform ist
PHP 5.2 (mal sehn wann auch die kleinen Server das auf schönere 5.3 umgestellt werden) und MySQL 5 vorgesehn

aktuell lauffähig:
- sperren vom direkten Ausführen von "Include"-Dateien
also aktuell noch fast alles im Scripts-Verzeichnis
(hier hatte ich auch erst die übliche Variante mit einer Konstante via DEFINE und jetzt wird es ohne externe Konstante über 'nen Stacktrace erledigt)
- Login (eingerichteter Benutzer ist "Admin" + "Pass")
- Logout
- die Sprachverwaltung
- eine dynamische Session (sie und ihr Keks wird nur erstellt, wenn sie nötig ist und ebenso auch wieder gelöscht)

noch nicht ausprobiert (aber hoffentlich lauffähig):
- Registrierung
- freischalten eines gesperrten/neuen Accounts
- neues Passwort, wenn vergessen
- die Basisverwaltung des Cronjobs
- viele Funktionen (aktuell noch, mal sehn ob ich das noch ändere), welche den Zugriff auf die grundlegenden Datenbank und Dateizugriffe bereitstellen

was noch fehlt:
- erstmal ist die Templateklasse noch nicht fertig
(drum auch billiges Echo+HTML in der Index.php)
- und erst dannach kann ich mich um den Rest kümmern
- nja und dann kommen noch so 10-20 weitere Dateien dazu, für die restlichen Funktionen (es soll ja alles klein bleiben)


Was den Dateiupload betrifft, da werden die Dateien entweder nicht von extern zugänglich sein (werden z.B. nur via readfile und vorherige Rechteprüfung freigegeben) und/oder ihre Dateinamen im Dateisystem werden geprüft und enthalten keine "bösen" Zeichen und auch die "schlimmen" Dateierweiterungen sind da nicht vorhanden (PHP hochladen und dann ausführen ist also nicht möglich)



bei den Sperren will ich noch was ändern
- also automatischer Überlasungsschutz kommt noch rein (falls unser Handy mal bei mir vorbeischaut und mit millionen Anfragen alles überlastet)
- damit verbunden auch eine etwas andere/bessere Art auf zuviele falsche Passworteingaben zu reagieren (aktuell wird nach 5 Versuchen der Account gesperrt und kann per Mail wieder freigeschaltet werden)


In dem Code hab ich die Debugausgabe aktiviert, also nicht über die Messages und den DEBUG OUTPUT wundern.
Und wie gesagt, aktuell geht es mir erstmal um die Sicherheit,
also voralem alles, welches in der Scripts/Functions.php in den Funktionen Main_GetConfigDefaults und besonders Main_Initialize behandelt wird.
Aus den Debugausgaben sollten auch alle gefährlichen Infos rausgefiltert werden.



Im Anhang liegen die Dateien
und hier hab ich noch 'nen Testserver mit den Dateien (9 MB)



Testserver:
- dort ist'n kleiner Apache, PHP 5.3 und MySQL 5.1 drinnen
- beim "installieren" entpackt der nur diese Module in sein Verzeichnis
- mein Projekt wird dann über http://localhost/Test/Scripts/Install.php5 eingerichtet
- und auf http://localhost/Test/Index.php5 liegt die Testseite

[edit]
ups, den Anhang vergessen

Hi!

Sehe ich das richtig, dass du keine Prepared-Statements verwendest?
Falls dem so ist, würde ich dir raten, es so bald als möglich zu tun.

Gründe u.a.:
*) Performance-Gewinn
*) Besserer Schutz vor SQL-Injections


Grüße, Frederic

Zitat von fkerber:

Sehe ich das richtig, dass du keine Prepared-Statements verwendest?

Gründe u.a.:
*) Besserer Schutz vor SQL-Injections

grade erst gesehn

Ja, im Code selber nutze ich sowas nicht, bzw. in einer etwas anderen Form vielleicht doch ... das ist aber alles in der Datenbankklasse integriert.

Einzig und alleine über die Feldnamen könnte man die "Schutzmaßnahmen", mittels bestimmter Prefixe absichtlich abschalten, aber von extern kommt man da nicht ran.


Ein Insert war z.B. so möglich.

markieren

Code:

function User_Set($Name, $Password, $Rights) {
  global $Config;
  $Config['DB']->Insert('UserTable', array(
    'Name'    => trim($Name),
    'Password' => $Password,
    'Rights'  => $Rights));
  return $Config['DB']->LastResult;
}

oder

markieren

Code:

return $Config['DB']->Insert('UserTable', array(
  'Name'    => trim($Name),
  'Password' => $Password,
  'Rights'  => $Rights));

oder auch sowas

markieren

Code:

if ($DB->Update('Irgendwas', array('feld' => 12345))) ...

hier würde der "feld ..."-Teil direkt übergeben
(hab halt keinen Query-Parser integriert)

markieren

Code:

if ($DB->Update('Irgendwas', 'feld = 12345')) ...
if ($DB->Update('Irgendwas', array('feld = 12345', 'x' => 0))) ...

man kann nur ein paar "grundlegende" Funktionen (mir reichen diese aber meistens aus) aufrufen, ihnen irgendwelche WHERE-, Feld- oder Daten-Listen mitgeben und dieses wird dann intern zusammengesetzt und z.B. über mysql_real_escape_string abgesichert/maskiert.

Zitat von himitsu:

zusammengesetzt und z.B. über mysql_real_escape_string abgesichert/maskiert.

So hab ich das bislang auch gemacht. Allerdings haben mich die PS überzeugt. Ich denke, ich werde meinen Code entsprechend umstellen.

Guck dir mal die PDO-Referenz an. Da siehst du was mit Prepared Statements gemeint ist

BTW (auch @Mithrandir): ich kann hier nur die Verwendung eines ORM (wie z.B. Doctrine) empfehlen.

Greetz
alcaeus

Zitat von alcaeus:

ich kann hier nur die Verwendung eines ORM (wie z.B. Doctrine) empfehlen.

Nu' hab ich ja schon 'ne fertige Datenbankklasse implementiert und komme eigentlich auch gut damit klar. Was wäre denn der Vorteil, ganz grob umrissen, den ich mit Doctrine hätte? Denn immerhin bedeutet das ja auch, dass ich mich in ein neues System einarbeiten müsste. Daher tue ich mich auch noch schwer, auf symfony umzuschwenken, zumal das System nahezu komplett ist.
Auf der anderen Seite könnte ich mir aber auch vorstellen, dass symfony einiges vereinfachen könnte. Hab mir mal das Tutorial mit der Job-Website durchgelesen. Grob erinnert mich so ein PHP-Framework an die VCL in Delphi.

Der Vorteil von Doctrine? Nu ja, nehmen wir an du hast eine Tabelle "Student" mit folgenden Spalten: id, lastname, firstname, birthday, matr.
Du hast entsprechend eine Klasse "Student" mit den Properties id, lastname, firstname, birthday, matr. Jedesmal wenn du Daten aus der Datenbank holst, werden automatisch die Objekte erzeugt, so dass du direkt damit arbeiten kannst:

markieren

PHP-Quellcode:

$student = Doctrine::getTable('Student')->findOneById($studentId);
if (!$student) {
// ...
}
$student->notifyAboutExams();

Das ist jetzt ein etwas stupides Beispiel, aber der Vorteil ist ganz eindeutig: jeder Datenbankzeile wird automatisch eine Objektinstanz zugewiesen.

Zusaetzlich gibts dann noch Plugins (Behaviors), welche dir Aufgaben abnehmen, z.B. das automatische Befuellen von Create- und Update-Timestamps, Soft-Delete, I18N (so dass du z.B. eine Kategorie hast, den Titel aber in beliebig viele Sprachen hinterlegst), uvm.

Zusammen mit Symfony kommt dann hinzu, dass dir diese Model-Klassen automatisch generiert werden, genauso wie die Formulare zur Datenmanipulation. Du hast also sehr schnell dein Skeleton anhand dessen du die Applikation zusammenbaust. Ist ein sehr interessanter Ansatz

Greetz
alcaeus

Hallo,

ich hoffe du hast nichts dagegen, wenn ich dein CMS mal etwas genauer auseinander nehme, nicht nur bezüglich Sicherheit.

• das gesamte Konzept deines CMS ist leider völlig veraltete Technik und alles andere als flexibel, tut mir Leid. Du wirst mit Sicherheit Probleme damit bekommen, wenn du das intensiver benutzen willst. MVC sollte das sein, was du suchst,
• Du verwendest globale Variablen und einige Konstanten ($Config oder R_GUEST), was wie auch in Delphi nicht gerade die feine englische Art ist.
• es sind einige Rechtschreib- oder Tippfehler zu finden. Main_Fialize() oder $_SESSION['Activ'].
• du mixt sehr extrem Groß- und Kleinschreibung. Das ist nicht hilfreich für Leute, die sich mit deiner API beschäftigen wollen. In den meisten Fällen hat sich ein "allesklein" für Variablen und Funktionen bewährt. Lediglich Klassen (dazu gleich mehr) schreibt man meistens in Camel_Case.
• eine functions.php ist keine gute Idee. Mag sein, dass die Ladegeschwindigkeit erhöht wird, weil alles in einer Datei steht, aber Versionierung, logische Trennung oder einfach nur Übersicht leiden schwer darunter.
• soweit ich dein CMS verstanden habe, muss man für jede neue Seite eine neue Datei anlegen und diese mit Kopf- und Fußzeilen ausstatten. Das artet in vielen Hacks und unnötigem Copy and Paste aus. Und wehe es ändert sich mal was ...
• Die wesentlichen Config-Einträge wie MySQL-Zugangsdaten stehen in Zeile 2090! Findest du nicht, dass derartig relevante Sachen eine eigene Datei verdient haben?
• es ist schön, dass du deinen Code (bzw die API) so einigermasen dokumentierst, aber mittlerweile gibt es sehr schöne inline-Dokumentationssysteme (phpDoc!), welche teilweise schon von Editoren unterstützt werden und wesentlich lesbarer und geläufiger (folglich einfacher) sind.
• du magst preg_match, oder? *g* Ein CMS sollte das möglichst vermeiden, denn es ist nicht gerade das Schnellste. Es gibt viele Stellen in deinem Code, die ein derartiges Monster an Funktionskraft gar nicht benötigen. Hier streiten sich viele (alcaeus wird gleich wieder kommen ... ^^), aber soweit ist das jedenfalls meine Meinung.
• Funtions.php5, Zeile 284; fummle doch bitte nich an den Dateirechten rum. 0777 ist ein böses Gerücht, völlig sinnlos und nicht Aufgabe des Script. Der Administrator selbst hat dafür zu sorgen, das Dateien die Rechte haben, die sie haben sollen. 0777 steht für Lese-Schreib- und Ausführrechte für jeden. Wozu muss ein Script ausgeführt werden (die werden interpretiert) und warum haben Andere Lese- und Schreibrechte darauf?
• und in Anbetracht dessen müsste man noch sagen, dass dein CMS wahrscheinlich nicht Windows-Server kompatibel ist (siehe Pfadnamen und diese chmods).
• bzgl. der Sicherheit ist mir sonst noch nichts aufgefallen. Mir fällt es schwer, relevante Stellen zu finden, denn was die Verzeichnisstruktur angeht, hast du dein Projekt sehr klein gehalten.

Was ich dir noch so als kleine Anregungen sagen möchte: Schau dir doch mal MVC an. Wenn man das geschickt umsetzt, hat man eine unglaubliche Flexibilität. OOP ist leider gar nicht verwendet worden, was dank PHPs Funktionsumfang sicher Spaß gemacht hätte. Dann wäre auch ein gutes URL-Management eine sinnvolle Sache. Lieber sollte man per htaccess immer die index.php aufrufen, die dann anhand der URL die richtige Seite includet. Und letztendlich werf ich mal noch den Begriff "Autoloader" in die Runde. Kapselt man alle Funktionalitäten in Klassen, so ist ein Autoloader eine feine Sache.

Ansonsten hast du dir sicher viel Mühe gegeben und eine Menge Arbeit da rein gesteckt. Du hast viel abstrakt gehalten und dich auf die Zukunft vorbereitet. Funktionieren wird das alles sicherlich, aber Spaß macht es später eher keinen mehr. PHP kann toller sein.

Wenn du noch Fragen zu den einzelnen Listenpunkten hast, dann nur her damit. Ich kann da gerne einiges genauer erläutern.

Liebe Grüße,
Valle

@Valle: was soll das denn heissen?

Ich geb auch noch bisserl Senf dazu, allerdings ohne mir die Muehe gemacht zu haben, den Code anzusehn. Security-Reviews gibts sind dann doch etwas aufwaendiger.
Ein paar allgemeine Kommentare kann ich dir geben:

• Valle hat Recht wenn er dir MVC ans Herz legt. Nein, eine Template-Engine macht deinen Code nicht MVC-konform. Ja, ein MVC-Framework hilft dir. Ich empfehle Symfony 1.4 und Doctrine (ist in symfony enthalten) als Framework-Loesung. Damit sind auch die von Frederic angesprochenen Prepared Statements abgehakt.
• index.php5 ist eine Sache die es nicht geben sollte. index.php ist schon eher richtig - FYI: es gibt nur eine unterstuetzte PHP-Version, und das ist PHP5. Hoster bei denen Dateien die Endung php5 haben muessen, damit PHP5 greift kannst du in die Tonne kloppen. Ausserdem ist .php die Standard-Erweiterung fuer PHP-Skripte. Ich aendere doch nicht meine Apache-Einstellungen (wo PHP bei Dateien mit der Endung .php greift) um ein CMS ausfuehren zu koennen. Sorry, aber is nicht.
• @Valle: camelCase funktioniert anders. Ich empfehle uebrigens einen Blick auf die PEAR oder ZF Coding-Guidelines - diese sind akzeptierter Standard. Da stehn noch ein paar andere sinnvolle Dinge drin.
• 777 ist toedlich. Ganz ehrlich: wenn www-data irgendwo schreiben soll, machs ueber die normalen Gruppen- und Benutzer-Berechtigungen von Linux. Es gibt keinen Grund warum der mysql-Benutzer in den Apache-Skripten rumfummeln darf oder warum Dateien gar ausfuehrbar sein sollen. 644 (Files) bzw. 755 (Directories) kombiniert mit chown und chgrp sollte dein bester Freund werden.
• Um das functions.php-Beispiel weiterzufuehren: wenn du die Funktionen logisch in Klassen kapselst und mit nem Autoloader arbeitest dann sparst du dir das unnoetige Laden von zig Funktionen. Das Parsen der PHP-Dateien kostet naemlich ohne Opcode-Cache richtig viel Zeit. Wenn dein Code nur eine von 20 Klassen braucht, wird auch nur die geladen, und dann auch erst sobald sie benoetigt wird. Guck dir hierzu mal spl_autoload() an. Da ist uebrigens noch ein Vorteil von MVC-Frameworks: diese implementieren sowas bereits
• Um Valle noch den Gefallen zu tun: bei der Performance-Optimierung eines grossen Ticketing-Systems haben wir uns entschlossen, ein is_preg-Flag in die Tabelle einzufuegen um so zu entscheiden ob wir per preg_match vergleichen oder per == wenn PREG nicht von Noeten ist. Das Ergebnis: der Code war nachher um den Faktor 20 (!) schneller. Muss ich noch mehr dazu sagen?

Mein ganz freundlich gemeinter Tipp: guck dir mal folgende Tutorials an: A gentle introduction to Symfony und Practical symfony (for Doctrine). Sobald du mal die Vorteile des Systems gespuert hast wirst du sie nicht mehr missen wollen (ich sag nur I18N, Test-Frameworks, Routing-Klassen, CRUD-Generation, etc. )

Greetz
alcaeus

Hallo,

ich möchte gerne noch die Sachen, welche alcaeus genannt hat unterstreichen. Hoster, die nur PHP in der Version 5 verwenden, wenn die Dateiendung .php5 ist, sind für die Tonne. Ich weiß dass es sowas noch gibt, aber der Einsatz eines längst nicht mehr unterstützen Software-Pakets ist ein Armutszeugnis für jeden Hoster...

Mit Doctrine habe ich bisher Erfahrung gemacht, was deren DB-Sachen betrifft (weiß jetzt nicht genau, ob da noch mehr dabei ist). Doctrine ist ein krasser Gegensatz zu herkömlichen Datenbank-Aktionen wie man sie so vielen PHP-Scripten kennt. Aber der Umstieg lohnt sich auf jeden Fall!

@alcaeus: Hätte ja nicht gedacht dass wir uns mal so einig sind. Hatten wir uns nicht mal darüber unterhalten, dass das einsparen solcher Regular Expressions deiner Meinung nach nichts bringen würde?

Liebe Grüße,
Valle