Suche Feedback - PHP MVC Framework (CodeMVC)

[flerovium]

Guten Tag,
ich habe gestern Abend damit angefangen aus Spaß ein MVC Framework in PHP zu schreiben, weil ich mich etwas tiefer damit beschäftigen will, als nur MVC Anwendungen auf Basis von z.B. CodeIgniter zu schreiben.
Da ich kein Ausgebildeter Programmierer bin, sondern das ganze im Moment noch nur als Hobby mache, hätte ich gerne Feedback zu meinem Code, was ich besser machen kann, was gut ist etc.

Hinweis: Die Klasse Security ist atm nur ein absoluter Rohbau und wird nicht so bleiben! Ich werde Filtermethoden für SQLi, XSS und RCE implementieren, so dass das Framework von einer soliden WAF geschützt sein wird ^^

Den Code findet ihr in meinem GIT-Repository: https://github.com/hackiosa/CodeMVC/tree/master/CodeMVC

Vielen Dank im Vorraus!

[tkausl]

        if ($config['sanitize'])
        {
            foreach ($_GET as $key => $value)
            {
                $_GET["$key"] = Security::WAF_Filter($value); 
            }
            
            foreach ($_POST as $key => $value)
            {
                $_POST["$key"] = Security::WAF_Filter($value);
            }
            
            foreach ($_COOKIE as $key => $value)
            {
                $_COOKIE["$key"] = Security::WAF_Filter($value);
            }
        } 

Ist vollkommener Schwachsinn. Und die " sind dort auch total unnötig.

$config['sanitize'] = true; // Do only set to false if you're insane! (you don't want to do that!) 

Sollte wohl eher lauten: Do only set to true if you want to fuck up all your input

require('/../config/config.php'); 

Das sehe ich an gefühlten 100 Stellen im Script. Optimalerweise sollte das nur genau einmalig passieren.
Außerdem wird es dir früher oder später krachen mit dem Slash am Anfang.

class Welcome_Controller 

Ich weiß nicht, ob das mit den Unterstrichen noch Zeitgemäß ist. Ich nutze CamelCase aber möglicherweise ist das doch noch ok.

In Zeiten von OOP in PHP nennt man seine Dateien auch nicht mehr .class.php, aber passt schon.

Außerdem nutzt du keine Namespaces. Gewöhn dich schon mal langsam an PSR-4.

Ansonsten.. Keine Ahnung. Ziemlich primitives Script. Kann nicht wirklich viel.
Aber dass jemand PHP mit Visual Studio schreibt hab ich auch noch nicht gesehn. lol.

[tr0y]

*kreisch*

Äh. Hi... Fang da an, bitte: http://php-de.github.io/jumpto/composer/

Mach da weiter:
- https://github.com/php-fig/fig-standards
- https://github.com/php-fig/fig-stand...ng-standard.md
- https://github.com/php-fig/fig-stand...-autoloader.md

Inspiration für Komponenten:
- https://github.com/nikic/FastRoute
- https://github.com/anandkunal/ToroPHP

Inspiration fürs Framework-Design ( micro > fullstack ):
- https://github.com/mikecao/flight ( http://flightphp.com/ )
- https://github.com/silexphp/Silex ( http://silex.sensiolabs.org/ )
- https://github.com/laravel/framework ( http://laravel.com )

Empfehlenswert: http://fabien.potencier.org/article/...ponents-part-1

Nomnom für Zwischendurch:
- http://de.wikipedia.org/wiki/Kompone...te_Entwicklung
- http://en.wikipedia.org/wiki/Event-driven_programming

"Not in this way" Implementierungen ( heutzutage ~2014 - subjektiv ):
- https://github.com/EllisLab/CodeIgniter/

Nicht unwichtig:
- http://de.wikipedia.org/wiki/Clean_Code
- Die Geschichte warum mir MVC auf den Sack geht, es lebe DCI, oder warum PHP sich noch weiter entwickeln muss.

Aber den Lolli in den Git-Comments find ich toll.

Viel Spass.

[Ma27]

Ich weiß nicht, ob das mit den Unterstrichen noch Zeitgemäß ist. Ich nutze CamelCase aber möglicherweise ist das doch noch ok.

In PHPSpec wird glaub ich auch noch das underscore-Zeugs empfohlen

Welcome_Controller sieht für mich nach einem dieser imo sinnfreien Workarounds für die Namespaces aus...

tr0y hat schon das meiste gesagt, hier noch ein paar Punkte:

in config Dateien wäre es imo am besten, einfach ein Array zurückzugeben, statt ein $config array oder so was zu nutzen (https://github.com/hackiosa/CodeMVC/...fig.php#L3-L14)

<?php
return [
    'key' => 'value',
    'key2' => 'value'
];

und dann:

<?php
$stuff = require __DIR__ . '/config/config.php';

https://github.com/hackiosa/CodeMVC/...oute.php#L3-L4
Sowas ist doch mega unflexibel, bei Symfony hat man da z.B. deutlich mehr Möglichkeiten: http://symfony.com/doc/current/book/routing.html

Deine "Template Engine" ist mega billig...
sieh dir mal Twig oder sowas an, da hat man deutlich mehr Möglichkeiten

https://github.com/hackiosa/CodeMVC/....class.php#L17
Wozu solcher Mist?! Nimm doch halbwegs sinnvolle Begriffe für Demos


Nun sollte ich vielleicht hinzufügen, dass MVC nicht die aller sauberste Architektur wiederspiegelt.
Ein Blick auf DCI und Clean COde (tr0y hats auch schon verlinkt) wäre mal ganz sinnvoll.
Zum Thema Clean Architecture gibt es auch noch ein ganz schickes Projekt: https://github.com/Opentribes/Core

LG

[flerovium]

        if ($config['sanitize'])
        {
            foreach ($_GET as $key => $value)
            {
                $_GET["$key"] = Security::WAF_Filter($value); 
            }
            
            foreach ($_POST as $key => $value)
            {
                $_POST["$key"] = Security::WAF_Filter($value);
            }
            
            foreach ($_COOKIE as $key => $value)
            {
                $_COOKIE["$key"] = Security::WAF_Filter($value);
            }
        } 

Ist vollkommener Schwachsinn. Und die " sind dort auch total unnötig.

$config['sanitize'] = true; // Do only set to false if you're insane! (you don't want to do that!) 

Sollte wohl eher lauten: Do only set to true if you want to fuck up all your input

Dann will ich sehen, wie du effektiv und effizient XSS und SQLi abwehrst.

require('/../config/config.php'); 

Das sehe ich an gefühlten 100 Stellen im Script. Optimalerweise sollte das nur genau einmalig passieren.
Außerdem wird es dir früher oder später krachen mit dem Slash am Anfang.

Gut, sehe ich ein

class Welcome_Controller 

Ich weiß nicht, ob das mit den Unterstrichen noch Zeitgemäß ist. Ich nutze CamelCase aber möglicherweise ist das doch noch ok.

Ich habe gelernt, dass das relativ egal, solange man das im ganzen Projekt einheitlich hält.

In Zeiten von OOP in PHP nennt man seine Dateien auch nicht mehr .class.php, aber passt schon.

Außerdem nutzt du keine Namespaces. Gewöhn dich schon mal langsam an PSR-4.

Werde ich mir angucken.

Ansonsten.. Keine Ahnung. Ziemlich primitives Script. Kann nicht wirklich viel.
Wie im Startpost steht, arbeite ich erst seit gestern daran un d ich hab seitdem einiges neugecodet. Des Weiteren wollte ich ja erstmal einen allgemeinen Eindruck, ob das bisher so vertretbar ist.

Aber dass jemand PHP mit Visual Studio schreibt hab ich auch noch nicht gesehn. lol.

Gibt eine ziemlich gute PHP Erweiterung für Visual Studio

*kreisch*

Äh. Hi... Fang da an, bitte: http://php-de.github.io/jumpto/composer/

Danke, kannte ich bisher nicht.

Mach da weiter:
- https://github.com/php-fig/fig-standards
- https://github.com/php-fig/fig-stand...ng-standard.md
- https://github.com/php-fig/fig-stand...-autoloader.md

Inspiration für Komponenten:
- https://github.com/nikic/FastRoute
- https://github.com/anandkunal/ToroPHP

Inspiration fürs Framework-Design ( micro > fullstack ):
- https://github.com/mikecao/flight ( http://flightphp.com/ )
- https://github.com/silexphp/Silex ( http://silex.sensiolabs.org/ )
- https://github.com/laravel/framework ( http://laravel.com )

Empfehlenswert: http://fabien.potencier.org/article/...ponents-part-1

Nomnom für Zwischendurch:
- http://de.wikipedia.org/wiki/Kompone...te_Entwicklung
- http://en.wikipedia.org/wiki/Event-driven_programming

"Not in this way" Implementierungen ( heutzutage ~2014 - subjektiv ):
- https://github.com/EllisLab/CodeIgniter/

Nicht unwichtig:
- http://de.wikipedia.org/wiki/Clean_Code
- Die Geschichte warum mir MVC auf den Sack geht, es lebe DCI, oder warum PHP sich noch weiter entwickeln muss.
Du willst mir also von einem voll OOP kompatiblen Pattern abraten und stattdessen soll ich ein Pattern verwenden das (wie es in der von dir genannten Quelle selbst steht!) nicht voll OOP kompatibel ist und das in einer OOP Sprache? LOL. Mal abgesehen davon, dass es in diesem Thema explizit um eine Implementierung von MVC geht und eine Diskussion MVC oder irgendein anderes Pattern vollkommen fehl am Platz ist! Wenn manche Leute ihre Wewechen mit MVC haben, dann ist mir das egal, denn ich komme mehr als gut damit zurecht!

Aber den Lolli in den Git-Comments find ich toll.
Danke, aber der stammt von Github .

Viel Spass.

[tkausl]

Dann will ich sehen, wie du effektiv und effizient XSS und SQLi abwehrst.

Bestimmt wehre ich SQLi nicht mit htmlentities ab. Alles was reinkommt einfach mal dadurch zu jagen bringt KEINE Sicherheit! SQLi wird vor der Query gehandhabt, optimalerweise mit Prepared Statements, und XSS erst bei der Ausgabe, nicht vorher!

[flerovium]

Bestimmt wehre ich SQLi nicht mit htmlentities ab.
Öhm, wenn du lesen könntest, wüsstest du, dass das atm nur der Platzhalter ist für den eigl. Code.
Alles was reinkommt einfach mal dadurch zu jagen bringt KEINE Sicherheit! SQLi wird vor der Query gehandhabt, optimalerweise mit Prepared Statements, und XSS erst bei der Ausgabe, nicht vorher!
Aha, wirklich sehr effizient! Wer garantiert mir, dass man da nicht das eine oder andere mal vergisst, Sicherheitsvorkehrungen zu treffen? Eine WAF ist einfach immernoch die beste und sicherste Lösung! Aber anscheinend hast du nicht so viel Ahnung von Websecurity.. Ne WAF ist einfach viel besser weil am Anfang gleich alles bösartige rausgeschmissen wird! Mehr als XSS und SQLi scheinst du auch nicht zu kennen?

[tkausl]

Aha, wirklich sehr effizient! Wer garantiert mir, dass man da nicht das eine oder andere mal vergisst, Sicherheitsvorkehrungen zu treffen? Eine WAF ist einfach immernoch die beste und sicherste Lösung! Aber anscheinend hast du nicht so viel Ahnung von Websecurity.. Ne WAF ist einfach viel besser weil am Anfang gleich alles bösartige rausgeschmissen wird!

Na klar, jag mal alles durch ne handvoll Funktionen, vieleicht bist du dann sicher

[Ma27]

kann das mal wer verschieben?
also wenn das jetzt auf diesem Niveau weitergeht, dann ist das Software-Design Forum imo die falsche Adresse

LG

[tr0y]

Ich rate dir zu nichts, du sollst dich LEDIGLICH AN DEN VON MIR GEPOSTETEN LINKS ORIENTIEREN. Wie ich dazu stehe ( zu DCI, MVC und PHP ) habe ich in dem Satz der aus den Links besteht beschrieben. Wenn du das weiter erörtert bekommen möchtest weil du irgendetwas davon nicht verstanden hast, kann ich das gerne tun.

Um das nochmal zu unterstreichen: Ich rate nicht davon ab MVC zu benutzen. Nur wirst du eine 100%-MVC Implementierung nicht gewährleisten können ( da diese weit über das Backend hinaus geht ), das zu erkennen und deine "Form" von MVC flexibel(er) zu gestalten, das ist dass, worauf ich dich bringen wollte.

[flerovium]

Gut dann tut es mir Leid @tr0y kam zumindest bei mir so rüber, als ob du mir was aufzwingen willst.

@tkausl Um einiges sicherer, als wenn ich an jeder Stelle wo User Input verwertet wird, von Hand meinen Ar*** rette!
Tut mir Leid, aber der Mensch ist fehlbar, daher sollte man in solchen Sicherheitsangelegenheiten einfach so viel wie möglich automatisieren lassen! Es gibt genug Gründe eine (anständig umgesetze, sprich Sachen wie UNunionION oder SELselectECT funktionieren nicht!) Web Application Firewall einzusetzen! Ich sehe keinen Grund, User Input nicht direkt zu sanitizen, wenn er ankommt! Resultiert imo auch in weniger Code! http://de.wikipedia.org/wiki/Web_Application_Firewall, read it.

[rkr]

Oh, stark.
Deine WebApplicationFirewall ist definitiv nicht das, was mein heute als guten Stil ansieht. Mag sein, dass du dich für sehr clever hälst, aber dein Code verrät dass du noch Jahre von dem Knowhow meiner Vorredner entfernt bist. Schau dir dir Links von Tr0y genau an.

PreparedStatements verhindern SQL-InjektionAttacken effektiv. Bis auf einen Fall, bei dem du eine StoredProcedure aufrufst, die selbst wieder ein SqlStatement zusammenbaut und ausführt und die eingehenden Werte nicht behandelt, kann nichts weiter passieren.

Meine Templateengine escapt jeden(!) Output, ausser ich will es nicht.

Dann ist die "Gefahr" immer Kontextabhängig. Wie willst du denn feststellen, was du später mit den Daten machst?

Eine Firewall kann keinen 100%igen Schutz bieten. Schließlich weisst du ja nicht, was man morgen alles machen kann - einmal eine Möglichkeit verschlafen (neues Html-Element, neues Html-Attribut, Utf8-Steuerzeichen), schon hast du dir eine Sicherheitslücke geschaffen.

Das was du da machst ist potentiell gefährlich - nicht nur dein bisheriger Code, sondern vor allem deine Einstellung.

[Asterixus]

Ein kleines Beispiel:

Ich will ein Bild mit deinem Benutzernamen anzeigen. Nur dummerweise bist du in Lisa verliebt und heißt Klaus. Daher hast du den Namen: Klaus<3Lisa gewählt.

Nun hast du aber mit Kanonen auf Spatzen geschossen: htmlspecialchars() (Klaus&lt;3Lisa) oder strip_tags() (wenn mich nicht alles täuscht: Klaus) wurde verwendet. Nur versteht die Grafikbibliothek deiner Wahl kein HTML und zeigt dann den verkrüppelten Namen an.

Santize-Aktionen sollten nur in dem Kontext ausgeführt werden, für den sie da sind. Wenn du anderer Meinung bist: am besten jeden String leeren, dann gehst du weniger Risiken ein.

[flerovium]

Oh, stark.
Deine WebApplicationFirewall ist definitiv nicht das, was mein heute als guten Stil ansieht. Mag sein, dass du dich für sehr clever hälst, aber dein Code verrät dass du noch Jahre von dem Knowhow meiner Vorredner entfernt bist. Schau dir dir Links von Tr0y genau an.
Dann will ich sehen wie meine Vorredner im Alter von 17 Jahren komplett alleine einen Emulator für eine Gaming Console entwickelt haben, oder etwas vom Schwierigkeitsgrad her vergleichbares, dann reden wir nochmal über Know-How

PreparedStatements verhindern SQL-InjektionAttacken effektiv. Bis auf einen Fall, bei dem du eine StoredProcedure aufrufst, die selbst wieder ein SqlStatement zusammenbaut und ausführt und die eingehenden Werte nicht behandelt, kann nichts weiter passieren.

Da gibt es nur ein Problem: Wenn das Framework auch normale SQL Queries (wie man sie von mysql_query kennt) anbieten soll / anbietet und dies von 3. Entwicklern unbedacht benutzt wird!

Meine Templateengine escapt jeden(!) Output, ausser ich will es nicht.
Und du denkst das ist das einzige Problem? Was machst du bei RCE, CSRF, LFI, RFI? Dass deine Templateengine alles escaped ist ja schön und gut, aber trotzdem gibt es noch genug andere Angriffsstellen, die man vielleicht nicht immer direkt aufm Plan hat, wenn man grad den betroffenen Code schreibt, da hilft eine WAF ungemein!

Dann ist die "Gefahr" immer Kontextabhängig. Wie willst du denn feststellen, was du später mit den Daten machst?

Eine Firewall kann keinen 100%igen Schutz bieten. Schließlich weisst du ja nicht, was man morgen alles machen kann - einmal eine Möglichkeit verschlafen (neues Html-Element, neues Html-Attribut, Utf8-Steuerzeichen), schon hast du dir eine Sicherheitslücke geschaffen.
Ach und wenn man alles selber macht, dann bietet das 100%igen Schutz? Der war gut

Das was du da machst ist potentiell gefährlich - nicht nur dein bisheriger Code, sondern vor allem deine Einstellung.

Aha, meine Einstellunge, alle möglichen Gefahren zu bannen, bevor sie zum Problem werden, ist gefährlich? Tell me more about how that sucks

[flerovium]

Ein kleines Beispiel:

Ich will ein Bild mit deinem Benutzernamen anzeigen. Nur dummerweise bist du in Lisa verliebt und heißt Klaus. Daher hast du den Namen: Klaus<3Lisa gewählt.

Nun hast du aber mit Kanonen auf Spatzen geschossen: htmlspecialchars() (Klaus&lt;3Lisa) oder strip_tags() (wenn mich nicht alles täuscht: Klaus) wurde verwendet. Nur versteht die Grafikbibliothek deiner Wahl kein HTML und zeigt dann den verkrüppelten Namen an.

Santize-Aktionen sollten nur in dem Kontext ausgeführt werden, für den sie da sind. Wenn du anderer Meinung bist: am besten jeden String leeren, dann gehst du weniger Risiken ein.

http://php.net/manual/de/function.ht...ars-decode.php
Kann man immernoch ggf. im Kontext falls Notwendig nutzen.