Sonntag, 29. Januar 2012, 01:37 UTC+1
Sie sind nicht angemeldet.
Lieber Besucher, herzlich willkommen bei: HackTheWeb Forum. Falls dies Ihr erster Besuch auf dieser Seite ist, lesen Sie sich bitte die Hilfe durch. Dort wird Ihnen die Bedienung dieser Seite näher erläutert. Darüber hinaus sollten Sie sich registrieren, um alle Funktionen dieser Seite nutzen zu können. Benutzen Sie das Registrierungsformular, um sich zu registrieren oder informieren Sie sich ausführlich über den Registrierungsvorgang. Falls Sie sich bereits zu einem früheren Zeitpunkt registriert haben, können Sie sich hier anmelden.
Sonntag, 25. Oktober 2009, 21:21
Sicherheitslücken und Verbesserungsvorschläge
Also nachdem ich von WarriorNight hierauf angesetzt worden bin, hab ich mich mal ein bissl durchgeklickt und Dinge die mir aufgefallen sind aufgeschrieben. Hoffe das wird mir nicht übel genommen.
Mal vorweg: Ich bin hier wesentlich detaillierter als dort. Bei dem dortigen Gegurke was sich Quelltext nennt, hatte ich nicht wirklich Lust alles aufzuzählen. Hier scheint sauberer gearbeitet zu werden.
Dann noch was: Ich hab einen ziemlich "konstruktiven Tonfall" drauf. Sprich ich zähl einfach ganz brutal auf was mir so auffällt. Hoffe das wird nicht falsch verstanden oder mir übel genommen. Außerdem kann es auch gut sein, dass ich Dinge erwähne, die sowieso schon in Planung oder unwichtig sind. Also nicht auf den Schlips treten lassen. Ich erwähne nur immer gerne alles um vollständig zu sein.
Aufbaudesign
Gut, also mal als kleiner Einstieg:
Ich bin früher schon öfters über dieses Spielprinzip gestolpert, hatte mich aber nie angemeldet, da ich mit schwarz als Hintergrundfarbe nicht viel anfangen kann (komm auch mit Weltraumspielen nicht klar
). Von dem her habe ich auch keine wirklich Vorkenntnisse, was den Spielablauf betrifft. Was mir also sofort aufgefallen ist: Es existieren keinerlei Hilfen.
Ich nehme mal an, dass die in der ToDo-Hierarchie ganz unten stehen und wenn sie noch in Planung sind, passt das auch so. Wollte nur mal erwähnt haben, dass ich mich unter normalen Umständen nicht angemeldet hätte, da ich nicht den Hauch einer Ahnung hatte worum es hier geht.
Fehler 2 der mir passiert ist: Ich wollte gleich mal ein Passwort anfordern und war kurz etwas verwirrt über die "User schon vorhanden"-Meldung. Ich hab recht schnell gecheckt, dass ich zum Registrierlink gehöre, aber vielleicht wäre es hier besser die Bezeichnung auf "Passwort vergessen" zu ändern.
Das bereits eingegebene Captcha find ich in diesem Kontext ziemlich cool. Find ich echt gut die Idee
. Bin mir aber nicht ganz sicher ob das nur eine Temporäre Lösung ist. Falls nicht, existiert beim Login eine Eingabeüberprüfung. Wäre nicht wirklich notwendig und ist verwirrend, wenn man die Loginseite doppelt offen hat und sich an der "alten" anmeldet.
Gut, im Spiel. Was tuh ich jetzt? Ich klick mich so durch, notier mir nebenbei, dass unter "Computer" statt den 3 "toten" Links (MoneyMarket, Überweisung ,Operation Center) eine Meldung wie "Noch nicht gebaut" oder so, ausgegeben werden könnte. Und jetzt? Ich klicke dahin, dorthin, meld mich am Alphaserver an und such dort weiter. Ich hab gute 2 min gebraucht, bis ich gecheckt hab dass der "Upgrade"-Link kein Kapitelreiter, sondern ein richtiger Link ist
Ich behaupte mal das liegt nur an meiner Blödheit, aber da das Spiel ja die breite Masse ansprechen soll, sollte es lieber eine "Dummheits"-Prävention inkludiert haben und zb die obere Leite wirklich eine Navi-Leiste umwandeln und einen Kapitelreiter namens "Computer Eigenschaften" hinzufügen
Was mir dann noch aufgefallen ist, ist das ingame kein Forumslink besteht. Ich war ne Zeitlang der Meinung es würde nur der Serverblog existieren. Stichwort Prävention undso
Oh, und hat die Scriptinfo-Tabelle auch spielerische Bedeutung? Für jemanden, der mit den Begriffen nichts anfangen kann, ist das wohl ziemlich verwirrend.
Quelltext
Nur ganz kurz durchgeblättert.
Mein Firebug gibt einen ganzen Haufen css-Meldungen aus.
Außerdem sollte man sich überlegen das Tabellendesgin in div-Design zu ändern. Schafft Übersichtlichkeit und spart Quelltext.
Dann hast du ein script-tag, dass du - wenn ich das richtig sehe - ruhig in den Head schieben kannst.
Des weiteren kannst du bei deinen form-tags oft auch das action-Attribut leer lassen anstatt die eh gerade Aufgerufene Url nochmal reinzusetzen. Auch die Fileangabe im Link ist überflüssig. Es genügt die Einfache Angabe der Get-Variablen.
Dann eine Grundfrage: Wofür ist das token-Attribut? Als Erweiterung für das phpSession-Cookie? Auf jeden Fall ist es nervig, wenn man die Get-Variablen untersuchen will
So, nun zum eigentlichen:
Also XSS, hab ich nirgends hinbekommen. Hier scheinen die Usereingaben ziemlich streng gefiltert zu werden. Generell hab ich nirgends eine direkte sql-Injektion hinbekommen. Nur bei Get-Variablen musst du aufpassen.
So war es mir zb möglich bei der Rangliste und beim Nachrichten schreiben per id-Variable in deine db zu kommen. Bei längeren Einträgen dauert es zwar länger, aber möglich ist es so trotzdem ohne weiteres deine db auszuspähen. Also auch hier unbedingt ein settype reinhauen.
Ach und, bei der Gelegenheit: Wie zum Teufel heißt deine Passwort-Spalte?? Ich hab alle Kombinationen ausprobiert, aber ich hab sie einfach nicht gefunden
Außerdem ist bei der Nachrichtenansicht auch eine ungeschützte id-Get-Variable. Ein recht unkomlpizierter Weg sich sämtliche Nachrichten durchzulesen. Nichts wirklich interessantes dabei
Und auch wenn ein settype dieses Problem sowieso beheben sollte, würde ich mir angewöhnen, dass die Anweisungen, die nicht von Uservariablen abhängig sind, vorher anzugeben.
Also
... WHERE $x = $y && id='$_GET[id]'
anstatt
... WHERE id='$_GET[id]' && $x = $y
Die Zauber-Injections-Zeichenfolge '--' (Kommentar-Ankündigung) wäre dann außer Kraft gesetzt.
Und natürlich dürfen die Anführungszeichen nicht vergessen werden. Die fehlen hier auch.
Um nur mal anzumerken:
Ich verwendete Anführungszeichen in den seltensten Fällen um Strings anzukündigen, sondern eigentlich nur um eventuelle Injektions zu relativieren.
So könnte zb ein Spieler sich mit dem Usernamen anmelden: 'users.name=0--'. Beim Anmeldescript sind die Anführungszeichen gesetzt, dadurch wird der Befehl nicht ausgeführt. Allerdings könnten bei einem Backup-Script, welches nur von Cronjobs ausgeführt wird, die Anführungszeichen nicht angegeben worden sein, da der Name ja aus der db ausgelesen wird und dadurch keine manipulierten Variablen erwartet werden.
Das Ergebnis wäre aber trotzdem, dass bei der Verwendung des Usernamens in Zusammenhang mit einem UPDATE-Befehl auf einen Schlag der gesamte Inhalt der db unwiederbringlich nicht mehr zu gebrauchen wäre.
(ja ich weiß, dass in Scripts, in welchen Strings in die db geschrieben werden die Anführungszeichen sowieso angeben werden müssen, da der Befehl sonst fehlerhaft wäre. Aber gehen wir mal von dem Fall aus, dass das Update-Script an einem Sonntag um 5 Uhr früh in halbtrunkenem Zustand geschrieben wurde und dass dieser eine mysql-Befehl nur unter bestimmten Voraussetzungen ausgeführt wird, die beim debugging nicht aufgetreten sind. Da nur der Server intern die Fehlermeldung bekommt, würde erst sehr spät auffallen, dass ein Fehler im Script existiert. Und eine Spieleranmeldung reicht schließlich aus...)
Oh und wenn ich das richtig sehe (ich sehe den Befehl zum ersten Mal) hat der Befehl mysql_escape_string denselben Effekt wie stripslashes, welcher durch magic_quotes=on sowieso überflüssig ist. Keine Ahnung was sich die php-Leudeln da gedacht haben.
Dann sind noch einige kleine Kleinigkeiten in deinem Quellcode, die mir aufgefallen sind:
Wie ich aus deinem (ziemlich coolen) Debug-Script erkennen kann, verwendest du öfters die Phrase:
(Anweisungg) ? $Variable = 1 : $Variable = 2;
Zeichenspaarender und (für mich persönlich) übersichtlicher wäre:
$Variable = (Anweisungg) ? 1 : 0;
Dann hast du in der Profilansicht keine Überprüfung, ob die User-id überhaupt existiert.
Bei dieser Gelegenheit erscheint übrigens auch ein Fehler im debugging-Script. Ganz am Schluss scheitert ein fopen-Befehl.
Bei den Nachrichten gibt es zwar eine Überprüfung, ob der User existiert, aber davor ist noch die "der ist gestört und redet mit sich selbst"-Überprüfung. Dadurch kommt es zu einem Notice (und ja: ich BIN gestört, was dagegen?
)
Oh, und xampp? Wurde das nachkonfiguriert? kA wie sicher und stabil die Standarteinstellungen sind...
Außerdem erscheint, wenn man auf dem Alpha-Server einen Account erstellt eine Fehlermeldung. Sonst scheint aber alles normal zu funzen...
Dann stört mich noch die Aufforderung zum Logout. Das hat mich bei gmx auch immer gestört. Ich arbeite alleine an meinem Computer. Die Gefahr, dass sich jemand, in dem Zeitraum in dem meine Session noch besteht an meinen Cookies oder Verlauf zu schaffen macht ist also sehr gering.
Ist nur so eine Prinzip-Sache. ich mag es nicht, wenn man mich an etwas erinnert, dass ich absichtlich außer Acht gelassen habe
Zwei letzte kleine Punkte gibt es noch:
Ohne Javascript ist es nicht möglich Aufträge ohne js in Auftrag zu geben oder abzubrechen. Eine Lösung wäre, bei den Links eine href-Angabe zu geben und das onClick-Event umändern in:
if(confirm("bla")) return true; else return false;
Dann müssten Spieler ohne js nur auf die Sicherheitsabfrage verzichten, könnten die Funktionen aber trotzdem (zb mit dem Handy) nutzen. Für "normal"-Spieler hätte das keine Änderung.
Bei den Buttons könnte man auch auf Links mit Hintergrundbildern umsteigen oder die Buttons jeweils mit formtags mit demselben onClick-Event umschließen.
Im großen und ganzen ist die Seite ziemlich gut.
Das Design ist hübsch (für nicht-schwarz-Allergiker) und sieht professionell aus, das Script sieht sauber geschrieben aus und Eingaben werden sehr gut gefiltert. Für jemanden der die db-Struktur nicht kennt und keine Serverantworten zur Verfügung hat, ist es ziemlich schwierig unerlaubt Daten erfolgreich zu manipulieren und für das das alles erst im Aufbau ist, muss ich sagen, großes Lob. Hier ist eine gute Grundstruktur geschaffen worden.
Im übrigen finde ich die Seite "gefühlsmäßig" gut. Hat eine gewisse Sympathie und auf den ersten Blick herrscht eine gute Kommunikation zwischen Admin und Community.
Nochmal großes Lob.
Ich hoffe meine Kritik kann verwertet werden und dass ich nicht zu streng war.
Ganz liebe Grüße
Jodli
Ps
So, um auch was davon zu haben: Hab heut erfahren, dass ich die Psychologie-Aufnahmepüfung nicht geschafft hab (nach 4 Monaten büffeln *grmbl*). Also bitte ich mal um eine Runde Mitleid
Mal vorweg: Ich bin hier wesentlich detaillierter als dort. Bei dem dortigen Gegurke was sich Quelltext nennt, hatte ich nicht wirklich Lust alles aufzuzählen. Hier scheint sauberer gearbeitet zu werden.
Dann noch was: Ich hab einen ziemlich "konstruktiven Tonfall" drauf. Sprich ich zähl einfach ganz brutal auf was mir so auffällt. Hoffe das wird nicht falsch verstanden oder mir übel genommen. Außerdem kann es auch gut sein, dass ich Dinge erwähne, die sowieso schon in Planung oder unwichtig sind. Also nicht auf den Schlips treten lassen. Ich erwähne nur immer gerne alles um vollständig zu sein.
Aufbaudesign
Gut, also mal als kleiner Einstieg:
Ich bin früher schon öfters über dieses Spielprinzip gestolpert, hatte mich aber nie angemeldet, da ich mit schwarz als Hintergrundfarbe nicht viel anfangen kann (komm auch mit Weltraumspielen nicht klar
). Von dem her habe ich auch keine wirklich Vorkenntnisse, was den Spielablauf betrifft. Was mir also sofort aufgefallen ist: Es existieren keinerlei Hilfen.Ich nehme mal an, dass die in der ToDo-Hierarchie ganz unten stehen und wenn sie noch in Planung sind, passt das auch so. Wollte nur mal erwähnt haben, dass ich mich unter normalen Umständen nicht angemeldet hätte, da ich nicht den Hauch einer Ahnung hatte worum es hier geht.
Fehler 2 der mir passiert ist: Ich wollte gleich mal ein Passwort anfordern und war kurz etwas verwirrt über die "User schon vorhanden"-Meldung. Ich hab recht schnell gecheckt, dass ich zum Registrierlink gehöre, aber vielleicht wäre es hier besser die Bezeichnung auf "Passwort vergessen" zu ändern.
Das bereits eingegebene Captcha find ich in diesem Kontext ziemlich cool. Find ich echt gut die Idee
. Bin mir aber nicht ganz sicher ob das nur eine Temporäre Lösung ist. Falls nicht, existiert beim Login eine Eingabeüberprüfung. Wäre nicht wirklich notwendig und ist verwirrend, wenn man die Loginseite doppelt offen hat und sich an der "alten" anmeldet.Gut, im Spiel. Was tuh ich jetzt? Ich klick mich so durch, notier mir nebenbei, dass unter "Computer" statt den 3 "toten" Links (MoneyMarket, Überweisung ,Operation Center) eine Meldung wie "Noch nicht gebaut" oder so, ausgegeben werden könnte. Und jetzt? Ich klicke dahin, dorthin, meld mich am Alphaserver an und such dort weiter. Ich hab gute 2 min gebraucht, bis ich gecheckt hab dass der "Upgrade"-Link kein Kapitelreiter, sondern ein richtiger Link ist
Ich behaupte mal das liegt nur an meiner Blödheit, aber da das Spiel ja die breite Masse ansprechen soll, sollte es lieber eine "Dummheits"-Prävention inkludiert haben und zb die obere Leite wirklich eine Navi-Leiste umwandeln und einen Kapitelreiter namens "Computer Eigenschaften" hinzufügen
Was mir dann noch aufgefallen ist, ist das ingame kein Forumslink besteht. Ich war ne Zeitlang der Meinung es würde nur der Serverblog existieren. Stichwort Prävention undso
Oh, und hat die Scriptinfo-Tabelle auch spielerische Bedeutung? Für jemanden, der mit den Begriffen nichts anfangen kann, ist das wohl ziemlich verwirrend.
Quelltext
Nur ganz kurz durchgeblättert.
Mein Firebug gibt einen ganzen Haufen css-Meldungen aus.
Außerdem sollte man sich überlegen das Tabellendesgin in div-Design zu ändern. Schafft Übersichtlichkeit und spart Quelltext.
Dann hast du ein script-tag, dass du - wenn ich das richtig sehe - ruhig in den Head schieben kannst.
Des weiteren kannst du bei deinen form-tags oft auch das action-Attribut leer lassen anstatt die eh gerade Aufgerufene Url nochmal reinzusetzen. Auch die Fileangabe im Link ist überflüssig. Es genügt die Einfache Angabe der Get-Variablen.
Dann eine Grundfrage: Wofür ist das token-Attribut? Als Erweiterung für das phpSession-Cookie? Auf jeden Fall ist es nervig, wenn man die Get-Variablen untersuchen will
So, nun zum eigentlichen:
Also XSS, hab ich nirgends hinbekommen. Hier scheinen die Usereingaben ziemlich streng gefiltert zu werden. Generell hab ich nirgends eine direkte sql-Injektion hinbekommen. Nur bei Get-Variablen musst du aufpassen.
So war es mir zb möglich bei der Rangliste und beim Nachrichten schreiben per id-Variable in deine db zu kommen. Bei längeren Einträgen dauert es zwar länger, aber möglich ist es so trotzdem ohne weiteres deine db auszuspähen. Also auch hier unbedingt ein settype reinhauen.
Ach und, bei der Gelegenheit: Wie zum Teufel heißt deine Passwort-Spalte?? Ich hab alle Kombinationen ausprobiert, aber ich hab sie einfach nicht gefunden
Außerdem ist bei der Nachrichtenansicht auch eine ungeschützte id-Get-Variable. Ein recht unkomlpizierter Weg sich sämtliche Nachrichten durchzulesen. Nichts wirklich interessantes dabei
Und auch wenn ein settype dieses Problem sowieso beheben sollte, würde ich mir angewöhnen, dass die Anweisungen, die nicht von Uservariablen abhängig sind, vorher anzugeben.
Also
... WHERE $x = $y && id='$_GET[id]'
anstatt
... WHERE id='$_GET[id]' && $x = $y
Die Zauber-Injections-Zeichenfolge '--' (Kommentar-Ankündigung) wäre dann außer Kraft gesetzt.
Und natürlich dürfen die Anführungszeichen nicht vergessen werden. Die fehlen hier auch.
Um nur mal anzumerken:
Ich verwendete Anführungszeichen in den seltensten Fällen um Strings anzukündigen, sondern eigentlich nur um eventuelle Injektions zu relativieren.
So könnte zb ein Spieler sich mit dem Usernamen anmelden: 'users.name=0--'. Beim Anmeldescript sind die Anführungszeichen gesetzt, dadurch wird der Befehl nicht ausgeführt. Allerdings könnten bei einem Backup-Script, welches nur von Cronjobs ausgeführt wird, die Anführungszeichen nicht angegeben worden sein, da der Name ja aus der db ausgelesen wird und dadurch keine manipulierten Variablen erwartet werden.
Das Ergebnis wäre aber trotzdem, dass bei der Verwendung des Usernamens in Zusammenhang mit einem UPDATE-Befehl auf einen Schlag der gesamte Inhalt der db unwiederbringlich nicht mehr zu gebrauchen wäre.
(ja ich weiß, dass in Scripts, in welchen Strings in die db geschrieben werden die Anführungszeichen sowieso angeben werden müssen, da der Befehl sonst fehlerhaft wäre. Aber gehen wir mal von dem Fall aus, dass das Update-Script an einem Sonntag um 5 Uhr früh in halbtrunkenem Zustand geschrieben wurde und dass dieser eine mysql-Befehl nur unter bestimmten Voraussetzungen ausgeführt wird, die beim debugging nicht aufgetreten sind. Da nur der Server intern die Fehlermeldung bekommt, würde erst sehr spät auffallen, dass ein Fehler im Script existiert. Und eine Spieleranmeldung reicht schließlich aus...)
Oh und wenn ich das richtig sehe (ich sehe den Befehl zum ersten Mal) hat der Befehl mysql_escape_string denselben Effekt wie stripslashes, welcher durch magic_quotes=on sowieso überflüssig ist. Keine Ahnung was sich die php-Leudeln da gedacht haben.
Dann sind noch einige kleine Kleinigkeiten in deinem Quellcode, die mir aufgefallen sind:
Wie ich aus deinem (ziemlich coolen) Debug-Script erkennen kann, verwendest du öfters die Phrase:
(Anweisungg) ? $Variable = 1 : $Variable = 2;
Zeichenspaarender und (für mich persönlich) übersichtlicher wäre:
$Variable = (Anweisungg) ? 1 : 0;
Dann hast du in der Profilansicht keine Überprüfung, ob die User-id überhaupt existiert.
Bei dieser Gelegenheit erscheint übrigens auch ein Fehler im debugging-Script. Ganz am Schluss scheitert ein fopen-Befehl.
Bei den Nachrichten gibt es zwar eine Überprüfung, ob der User existiert, aber davor ist noch die "der ist gestört und redet mit sich selbst"-Überprüfung. Dadurch kommt es zu einem Notice (und ja: ich BIN gestört, was dagegen?
)Oh, und xampp? Wurde das nachkonfiguriert? kA wie sicher und stabil die Standarteinstellungen sind...
Außerdem erscheint, wenn man auf dem Alpha-Server einen Account erstellt eine Fehlermeldung. Sonst scheint aber alles normal zu funzen...
Dann stört mich noch die Aufforderung zum Logout. Das hat mich bei gmx auch immer gestört. Ich arbeite alleine an meinem Computer. Die Gefahr, dass sich jemand, in dem Zeitraum in dem meine Session noch besteht an meinen Cookies oder Verlauf zu schaffen macht ist also sehr gering.
Ist nur so eine Prinzip-Sache. ich mag es nicht, wenn man mich an etwas erinnert, dass ich absichtlich außer Acht gelassen habe
Zwei letzte kleine Punkte gibt es noch:
Ohne Javascript ist es nicht möglich Aufträge ohne js in Auftrag zu geben oder abzubrechen. Eine Lösung wäre, bei den Links eine href-Angabe zu geben und das onClick-Event umändern in:
if(confirm("bla")) return true; else return false;
Dann müssten Spieler ohne js nur auf die Sicherheitsabfrage verzichten, könnten die Funktionen aber trotzdem (zb mit dem Handy) nutzen. Für "normal"-Spieler hätte das keine Änderung.
Bei den Buttons könnte man auch auf Links mit Hintergrundbildern umsteigen oder die Buttons jeweils mit formtags mit demselben onClick-Event umschließen.
Im großen und ganzen ist die Seite ziemlich gut.
Das Design ist hübsch (für nicht-schwarz-Allergiker
) und sieht professionell aus, das Script sieht sauber geschrieben aus und Eingaben werden sehr gut gefiltert. Für jemanden der die db-Struktur nicht kennt und keine Serverantworten zur Verfügung hat, ist es ziemlich schwierig unerlaubt Daten erfolgreich zu manipulieren und für das das alles erst im Aufbau ist, muss ich sagen, großes Lob. Hier ist eine gute Grundstruktur geschaffen worden.Im übrigen finde ich die Seite "gefühlsmäßig" gut. Hat eine gewisse Sympathie und auf den ersten Blick herrscht eine gute Kommunikation zwischen Admin und Community.
Nochmal großes Lob.
Ich hoffe meine Kritik kann verwertet werden und dass ich nicht zu streng war.
Ganz liebe Grüße
Jodli
Ps
So, um auch was davon zu haben: Hab heut erfahren, dass ich die Psychologie-Aufnahmepüfung nicht geschafft hab (nach 4 Monaten büffeln *grmbl*). Also bitte ich mal um eine Runde Mitleid
Montag, 26. Oktober 2009, 08:41
Also nachdem ich von WarriorNight hierauf angesetzt worden bin, hab ich mich mal ein bissl durchgeklickt und Dinge die mir aufgefallen sind aufgeschrieben. Hoffe das wird mir nicht übel genommen.
Mal vorweg: Ich bin hier wesentlich detaillierter als dort. Bei dem dortigen Gegurke was sich Quelltext nennt, hatte ich nicht wirklich Lust alles aufzuzählen. Hier scheint sauberer gearbeitet zu werden.
Dann noch was: Ich hab einen ziemlich "konstruktiven Tonfall" drauf. Sprich ich zähl einfach ganz brutal auf was mir so auffällt. Hoffe das wird nicht falsch verstanden oder mir übel genommen. Außerdem kann es auch gut sein, dass ich Dinge erwähne, die sowieso schon in Planung oder unwichtig sind. Also nicht auf den Schlips treten lassen. Ich erwähne nur immer gerne alles um vollständig zu sein.
Mach dir keine Sorgen. Wenn ich dich bitte mir aufs Maul zu haun dann jammer ich nicht über die daraus reslutierenden Wunden. Schlag zu, je härter desto besser :-)
Zitat
Aufbaudesign
Gut, also mal als kleiner Einstieg:
Ich bin früher schon öfters über dieses Spielprinzip gestolpert, hatte mich aber nie angemeldet, da ich mit schwarz als Hintergrundfarbe nicht viel anfangen kann (komm auch mit Weltraumspielen nicht klar
Ich nehme mal an, dass die in der ToDo-Hierarchie ganz unten stehen und wenn sie noch in Planung sind, passt das auch so. Wollte nur mal erwähnt haben, dass ich mich unter normalen Umständen nicht angemeldet hätte, da ich nicht den Hauch einer Ahnung hatte worum es hier geht.
In der Tat steht die Dokumentation eher hinten an. Das hat den Hauptgrund das trotz Planung in der umsetzung der Features und deren Bedienung sehr oft noch Änderungen gemacht werden. Die Dokumentation wird bald starten und dann neben der Entwicklung herlaufen. Bisher hat es wenig Sinn gemacht die unfertigen Elemnte zu beschreiben. Nach Fertigstellung ist auch ein interaktives Tutorial geplant das den Spieler auf die wichtigsten Dinger hinweist und Ihn etwas an die Hand nimmt.
Fehler 2 der mir passiert ist: Ich wollte gleich mal ein Passwort anfordern und war kurz etwas verwirrt über die "User schon vorhanden"-Meldung. Ich hab recht schnell gecheckt, dass ich zum Registrierlink gehöre, aber vielleicht wäre es hier besser die Bezeichnung auf "Passwort vergessen" zu ändern.
AUA - und Danke :-)
Das bereits eingegebene Captcha find ich in diesem Kontext ziemlich cool. Find ich echt gut die Idee
Jo, its not a Bug its a Feature. Ich log mehrmals am Tag ein und hab einfach kein Bock jedesmal das Captcha einzugeben. zur zeit gehts auch mehr ums Testen als ums ehrliche Spielen(sind zu diesem zweck ja auch Cheats eingebaut) von daher...
Das mit der Eingabeüberprüfung und der doppelt geöffneten Startseite habe ich nicht verstanden
Gut, im Spiel. Was tuh ich jetzt? Ich klick mich so durch, notier mir nebenbei, dass unter "Computer" statt den 3 "toten" Links (MoneyMarket, Überweisung ,Operation Center) eine Meldung wie "Noch nicht gebaut" oder so, ausgegeben werden könnte. Und jetzt? Ich klicke dahin, dorthin, meld mich am Alphaserver an und such dort weiter. Ich hab gute 2 min gebraucht, bis ich gecheckt hab dass der "Upgrade"-Link kein Kapitelreiter, sondern ein richtiger Link ist
Ich behaupte mal das liegt nur an meiner Blödheit, aber da das Spiel ja die breite Masse ansprechen soll, sollte es lieber eine "Dummheits"-Prävention inkludiert haben und zb die obere Leite wirklich eine Navi-Leiste umwandeln und einen Kapitelreiter namens "Computer Eigenschaften" hinzufügen
Jep ich weiß was du meinst und das ist mir vor ein paar Tagen auch schon aufgefallen. Alleine aus Gründen der Bedienung wird das noch hinreichend geändert.Ist schongeplant aber nicht umgestezt.
Was mir dann noch aufgefallen ist, ist das ingame kein Forumslink besteht. Ich war ne Zeitlang der Meinung es würde nur der Serverblog existieren. Stichwort Prävention undso
siehe oben :-) das sind so Kleinigkeiten die fallen einem erst auf wenn man wirklich als Anwender dabei ist. Hatte den Forumslink ehrlich gesagt schon wieder vergessen, den zieh ich heute nach.
Oh, und hat die Scriptinfo-Tabelle auch spielerische Bedeutung? Für jemanden, der mit den Begriffen nichts anfangen kann, ist das wohl ziemlich verwirrend.
Nein die Scriptinfo hat keine spielerische Bedeutung ist aber für u.a. für das Bugreporting entscheidend.
Quelltext
Nur ganz kurz durchgeblättert.
Mein Firebug gibt einen ganzen Haufen css-Meldungen aus.
jep, das sind die colerierten Scrollbalken für den IE. das gibts im Firefox nicht. Es gibt auch noch keine Browserweiche für CSS oder JS.
Außerdem sollte man sich überlegen das Tabellendesgin in div-Design zu ändern. Schafft Übersichtlichkeit und spart Quelltext.
Ja ist mir bewusst, bin aber leider CSS Krüppel und muss das später nachziehen. Sollte aber nicht der Aufwand werden da ich ne relativ gute Trennung von HTML und PHP habe.
Dann hast du ein script-tag, dass du - wenn ich das richtig sehe - ruhig in den Head schieben kannst.
Des weiteren kannst du bei deinen form-tags oft auch das action-Attribut leer lassen anstatt die eh gerade Aufgerufene Url nochmal reinzusetzen. Auch die Fileangabe im Link ist überflüssig. Es genügt die Einfache Angabe der Get-Variablen.
Jo, habe mich aber aus Übersichtsgründen im Template zu der Variante entschieden.
Dann eine Grundfrage: Wofür ist das token-Attribut? Als Erweiterung für das phpSession-Cookie? Auf jeden Fall ist es nervig, wenn man die Get-Variablen untersuchen will
hihi, jo normale user sollen die URL nicht untersuchen wollen :-) das Token ist ein SharedSecret zur prävention von CrossSiteScripting
So, nun zum eigentlichen:
Also XSS, hab ich nirgends hinbekommen. Hier scheinen die Usereingaben ziemlich streng gefiltert zu werden. Generell hab ich nirgends eine direkte sql-Injektion hinbekommen. Nur bei Get-Variablen musst du aufpassen.
So war es mir zb möglich bei der Rangliste und beim Nachrichten schreiben per id-Variable in deine db zu kommen. Bei längeren Einträgen dauert es zwar länger, aber möglich ist es so trotzdem ohne weiteres deine db auszuspähen. Also auch hier unbedingt ein settype reinhauen.
Ach und, bei der Gelegenheit: Wie zum Teufel heißt deine Passwort-Spalte?? Ich hab alle Kombinationen ausprobiert, aber ich hab sie einfach nicht gefunden
Es gibt keine Passwortspalte :-) Jedenfalls nicht in der Datenbank in der du rumgewurschtelt hast :-)
Montag, 26. Oktober 2009, 08:41
Außerdem ist bei der Nachrichtenansicht auch eine ungeschützte id-Get-Variable. Ein recht unkomlpizierter Weg sich sämtliche Nachrichten durchzulesen. Nichts wirklich interessantes dabei
Und auch wenn ein settype dieses Problem sowieso beheben sollte, würde ich mir angewöhnen, dass die Anweisungen, die nicht von Uservariablen abhängig sind, vorher anzugeben.
Also
... WHERE $x = $y && id='$_GET[id]'
anstatt
... WHERE id='$_GET[id]' && $x = $y
Die Zauber-Injections-Zeichenfolge '--' (Kommentar-Ankündigung) wäre dann außer Kraft gesetzt.
Und natürlich dürfen die Anführungszeichen nicht vergessen werden. Die fehlen hier auch.
Um nur mal anzumerken:
Ich verwendete Anführungszeichen in den seltensten Fällen um Strings anzukündigen, sondern eigentlich nur um eventuelle Injektions zu relativieren.
So könnte zb ein Spieler sich mit dem Usernamen anmelden: 'users.name=0--'. Beim Anmeldescript sind die Anführungszeichen gesetzt, dadurch wird der Befehl nicht ausgeführt. Allerdings könnten bei einem Backup-Script, welches nur von Cronjobs ausgeführt wird, die Anführungszeichen nicht angegeben worden sein, da der Name ja aus der db ausgelesen wird und dadurch keine manipulierten Variablen erwartet werden.
Das Ergebnis wäre aber trotzdem, dass bei der Verwendung des Usernamens in Zusammenhang mit einem UPDATE-Befehl auf einen Schlag der gesamte Inhalt der db unwiederbringlich nicht mehr zu gebrauchen wäre.
(ja ich weiß, dass in Scripts, in welchen Strings in die db geschrieben werden die Anführungszeichen sowieso angeben werden müssen, da der Befehl sonst fehlerhaft wäre. Aber gehen wir mal von dem Fall aus, dass das Update-Script an einem Sonntag um 5 Uhr früh in halbtrunkenem Zustand geschrieben wurde und dass dieser eine mysql-Befehl nur unter bestimmten Voraussetzungen ausgeführt wird, die beim debugging nicht aufgetreten sind. Da nur der Server intern die Fehlermeldung bekommt, würde erst sehr spät auffallen, dass ein Fehler im Script existiert. Und eine Spieleranmeldung reicht schließlich aus...)
Oh und wenn ich das richtig sehe (ich sehe den Befehl zum ersten Mal) hat der Befehl mysql_escape_string denselben Effekt wie stripslashes, welcher durch magic_quotes=on sowieso überflüssig ist. Keine Ahnung was sich die php-Leudeln da gedacht haben.
ok danke werde ich mir ansehen. magic_quotes habe ich off.
Dann sind noch einige kleine Kleinigkeiten in deinem Quellcode, die mir aufgefallen sind:
Wie ich aus deinem (ziemlich coolen) Debug-Script erkennen kann, verwendest du öfters die Phrase:
(Anweisungg) ? $Variable = 1 : $Variable = 2;
Zeichenspaarender und (für mich persönlich) übersichtlicher wäre:
$Variable = (Anweisungg) ? 1 : 0;
Dann hast du in der Profilansicht keine Überprüfung, ob die User-id überhaupt existiert.
Bei dieser Gelegenheit erscheint übrigens auch ein Fehler im debugging-Script. Ganz am Schluss scheitert ein fopen-Befehl.
Bei den Nachrichten gibt es zwar eine Überprüfung, ob der User existiert, aber davor ist noch die "der ist gestört und redet mit sich selbst"-Überprüfung. Dadurch kommt es zu einem Notice (und ja: ich BIN gestört, was dagegen?
Jo, ich war ziemlich schlampig was die Prüfung der Eingangsvariabeln angeht. Es sind ein paar Dinge vernachglässigt worden die ich für die nächste Zeit zur Ausbesserung geplant hatte. Für die Typisierung von Usereingaben hab ich ne Idee muss aber sehen ob das umzusetzen ist.
Oh, und xampp? Wurde das nachkonfiguriert? kA wie sicher und stabil die Standarteinstellungen sind...
ja, xampp in der standartinstallation is nicht zu gebrauchen...
Außerdem erscheint, wenn man auf dem Alpha-Server einen Account erstellt eine Fehlermeldung. Sonst scheint aber alles normal zu funzen...
Dann stört mich noch die Aufforderung zum Logout. Das hat mich bei gmx auch immer gestört. Ich arbeite alleine an meinem Computer. Die Gefahr, dass sich jemand, in dem Zeitraum in dem meine Session noch besteht an meinen Cookies oder Verlauf zu schaffen macht ist also sehr gering.
Ist nur so eine Prinzip-Sache. ich mag es nicht, wenn man mich an etwas erinnert, dass ich absichtlich außer Acht gelassen habe
Zwei letzte kleine Punkte gibt es noch:
Ohne Javascript ist es nicht möglich Aufträge ohne js in Auftrag zu geben oder abzubrechen. Eine Lösung wäre, bei den Links eine href-Angabe zu geben und das onClick-Event umändern in:
if(confirm("bla")) return true; else return false;
Dann müssten Spieler ohne js nur auf die Sicherheitsabfrage verzichten, könnten die Funktionen aber trotzdem (zb mit dem Handy) nutzen. Für "normal"-Spieler hätte das keine Änderung.
Bei den Buttons könnte man auch auf Links mit Hintergrundbildern umsteigen oder die Buttons jeweils mit formtags mit demselben onClick-Event umschließen.
JS ist Voraussetzung, einige spätere Spielelemente werden das auch benötigen. Stimmt schon das ich meine kleine Mach_mir_nen_button Funktion sehr gerne vergewaltige :-)
Im großen und ganzen ist die Seite ziemlich gut.
Das Design ist hübsch (für nicht-schwarz-Allergiker
Im übrigen finde ich die Seite "gefühlsmäßig" gut. Hat eine gewisse Sympathie und auf den ersten Blick herrscht eine gute Kommunikation zwischen Admin und Community.
Nochmal großes Lob.
Ich hoffe meine Kritik kann verwertet werden und dass ich nicht zu streng war.
neee quatsch, Danke für das Lob der Grundstruktur gegenüber. Design sollte kein Hinderungsgrund sein denn die Unterstütung von Grafikpaketen ist bereits implementiert, macht aber derzeit keinen Sinn da das Pack alle paar Tage geändert werden müsste.
Du hast mir auf jeden Fall sehr geholfen und ich werde mir deine Schreibe auch die nächsten Tage zur Brust nehmen.
Ganz liebe Grüße
Jodli
Ps
So, um auch was davon zu haben: Hab heut erfahren, dass ich die Psychologie-Aufnahmepüfung nicht geschafft hab (nach 4 Monaten büffeln *grmbl*). Also bitte ich mal um eine Runde Mitleid
son scheißß... na wer weiß wofür es gut ist. Mitleid gibts nicht, aber die besten Wünsche und Viel Glück für die Zukunft..
Dienstag, 3. November 2009, 16:35
Oh und wenn ich das richtig sehe (ich sehe den Befehl zum ersten Mal) hat der Befehl mysql_escape_string denselben Effekt wie stripslashes, welcher durch magic_quotes=on sowieso überflüssig ist. Keine Ahnung was sich die php-Leudeln da gedacht haben.
Ach ja genau da war noch was. Was ich sagen woltle zu diesem Satz:
Wer verwendet denn um alles in der Welt magic_quotes=on ?
Es gibt einige Grüne warum man das nicht tun sollte. Eein ausgewähltes escaping wesentlich performanter als ein magic_quote=on
man muss ja selten alles escapen was übergeben wird, und wie oft muss man unnötig escaptes später wieder per stripslashes zurücknehmen?
magic_quotes ist was für kindergartencoder. Und ich wünsche alles leuten die sich mit magic_quotes das leben leicht gemacht haben viel spass
bei der portierung zu PHP 6.0. magic quotes ist dort nämlich deprecated. also schonmal schön nachprogrammieren :-) dann wird vieleicht auch klar wie dumm diese funktion ist wenn man sich das mal anschaut, wie oft dort unnötig strings manipuliert werden.
und mysql_esacpe_string ist nicht das selbe wie stripslashes und auch nicht das selbe wie addslashes :-)
Dienstag, 3. November 2009, 20:24
Also erstmal: Mein Ausruf über die "php-Leudlen" war auf den - soweit ich das sehe - für-mysql-escaping-unzureichenden-Befehl (mysql_escape_string) und nicht auf magic_quotes bezogen. Dass magic_quotes nicht unbedingt notwenig sind, ist mir bewusst.
Aber ich weiß nicht warum alle so über magic_quotes fluchen. Klar ist es nervig der Funktion hinterherrennen zu müssen, aber als dumm würde ich die sie trotzdem nicht bezeichnen.
Wenn man es sich performance-technisch leisten kann und weiß was man tut, sehe ich keinen Grund, warum man magic_quotes nicht aktivieren sollte.
Grundsätzlich finde ich das Schema "Ich escape alles, außer wenn du mich explizit davon abhältst" Sicherheitstechnisch sehr vernünftig.
Sicherheitslecks sind nun mal so ziemlich überall zu finden. Das passiert dem erfahrensten Scripter. Und im Normalfall fällt sowas erst auf, wenn es zu spät ist. Da ist es ganz praktisch, wenns zusätzlich eine "automatisierte" Verarbeitungsmethode gibt, die zumindest den Schaden begrenzen kann.
Nimm beispielsweise die Situation mit der id-Variable bei den Nachrichten in htw. Hier wurde nur eine einzige Überprüfung vergessen. Aber ich hätte durchaus die Möglichkeit gehabt mit einem verschachtelten Select eigene Files zu erstellen um mir somit den ganzen Server zu öffnen. Aber mit magic_quotes und gesetzten Anführungszeichen hätte ich hier keine Chance auf injection gehabt.
Diejenigen die die Funktion wirklich als Escape-Ersatz verwenden, werden auch weiterhin zu blöd sein, Eingaben zu filtern und die anderen, die wissen was sie tun, werden trotzdem Fehler machen.
Von dem was da im Netz rum spukt bezweifle ich, dass wir schon so weit sind die Funktion standartmäßig rauszunehmen.
naja. Vielleicht hab ich auch nur zu alte Bücher gelesen. Lass mich gerne überzeugen. Kann gut sein, dass mein Wissen in der Hinsicht veraltet ist.
Oh, addslashes und stripslashes.... Ist doch fast das gleiche
jaja du hast ja recht...
Ich check den Unterschied zwischen mysql_esacpe_string und addslashes aber trotzdem noch nicht. Was ersetzt das noch außer " und ' ?
Aber ich weiß nicht warum alle so über magic_quotes fluchen. Klar ist es nervig der Funktion hinterherrennen zu müssen, aber als dumm würde ich die sie trotzdem nicht bezeichnen.
Wenn man es sich performance-technisch leisten kann und weiß was man tut, sehe ich keinen Grund, warum man magic_quotes nicht aktivieren sollte.
Grundsätzlich finde ich das Schema "Ich escape alles, außer wenn du mich explizit davon abhältst" Sicherheitstechnisch sehr vernünftig.
Sicherheitslecks sind nun mal so ziemlich überall zu finden. Das passiert dem erfahrensten Scripter. Und im Normalfall fällt sowas erst auf, wenn es zu spät ist. Da ist es ganz praktisch, wenns zusätzlich eine "automatisierte" Verarbeitungsmethode gibt, die zumindest den Schaden begrenzen kann.
Nimm beispielsweise die Situation mit der id-Variable bei den Nachrichten in htw. Hier wurde nur eine einzige Überprüfung vergessen. Aber ich hätte durchaus die Möglichkeit gehabt mit einem verschachtelten Select eigene Files zu erstellen um mir somit den ganzen Server zu öffnen. Aber mit magic_quotes und gesetzten Anführungszeichen hätte ich hier keine Chance auf injection gehabt.
Diejenigen die die Funktion wirklich als Escape-Ersatz verwenden, werden auch weiterhin zu blöd sein, Eingaben zu filtern und die anderen, die wissen was sie tun, werden trotzdem Fehler machen.
Von dem was da im Netz rum spukt bezweifle ich, dass wir schon so weit sind die Funktion standartmäßig rauszunehmen.
naja. Vielleicht hab ich auch nur zu alte Bücher gelesen. Lass mich gerne überzeugen. Kann gut sein, dass mein Wissen in der Hinsicht veraltet ist.
Oh, addslashes und stripslashes.... Ist doch fast das gleiche
jaja du hast ja recht...
Ich check den Unterschied zwischen mysql_esacpe_string und addslashes aber trotzdem noch nicht. Was ersetzt das noch außer " und ' ?
Freitag, 27. November 2009, 23:10
Ich hab gute 2 min gebraucht, bis ich gecheckt hab dass der "Upgrade"-Link kein Kapitelreiter, sondern ein richtiger Link ist
Ich behaupte mal das liegt nur an meiner Blödheit, aber da das Spiel ja die breite Masse ansprechen soll, sollte es lieber eine "Dummheits"-Prävention inkludiert haben und zb die obere Leite wirklich eine Navi-Leiste umwandeln und einen Kapitelreiter namens "Computer Eigenschaften" hinzufügen
DONE