• Jetzt anmelden. Es dauert nur 2 Minuten und ist kostenlos!

TMS (Termine Messenger System) bewerten

B

brainiac

Neues Mitglied
Hi! Ich habe ein TMS (neu erfundenes Wort) entwickelt und nun möchte ich wissen, wie das Ding so ist. Es gibt keine Online-aufführung sondern nur zum Download uns AUSPROBIEREN auf einem lokalen Server. Nach ca. 5-10 Downloads werde ich es wieder stoppen. Ich mag es nicht, wenn Kundenprojekte im Internet überall herumfliegen. Deswegen bitte ich die Downloader, dass sie nach dem Testen ein Statement im Forum dazu abgeben und auch es nicht veröffentlichen. Außerdem ist es ratsam bei der Installation die Anleitung zu lesen, ich mag zwar selber auch keine Anleitungen, aber ansonsten bekommt ihr eh nur Fehlermeldungen....

Viele Grüße
Jonathan

WICHTIG! Mir ist ein Programmierfehler passiert!
in mysql_inc.php hinter den "" bei den Teilen, wo ihr was einsetzen sollt, ein ; setzen!

**EDIT** Irgendwie funktioniert das hier nicht. Mein Zip-Archiv ist doppelt so groß wie erlaubt :D Ich lade es dann eben auf meinen eigenen Server hoch :D
Jonathan Lusky - Meine Neue HP Dort findet ihr schon einen Downloadlink
 
Zuletzt bearbeitet:
Hallo Jonathan.

Ein wenig konstruktive Kritik:

- Deine gesamte Anwendung ist anfällig für SQL-Injections, weil du Nutzereingaben (POST und GET) direkt in die Queries einsetzt. Ein Beispiel: Wenn ich "1' OR 1 -- " (ohne die äußeren Anführungszeichen) als POST['delete'] an nachrichten.php schicke, räume ich damit die gesamte PN-Tabelle leer. (Zumindest dann, wenn auf dem Server "magic quotes" deaktiviert sind, was der Normalfall und bald wohl der einzige Fall ist.)

- Der erste Punkt gilt übrigens für alle Daten, die direkt oder indirekt vom Benutzer stammen. Nimm mal diesen Ausschnitt:

PHP: 
$sql_2 = mysql_query("UPDATE t_user SET passwort = '".$pw_1."' WHERE username LIKE
    '".$_SESSION["username"]."'");

Wenn der Benutzername passend gewählt wird, lässt sich hier mit einem ähnlichen Trick wie beim vorherigen Punkt jedes Benutzerpasswort auf $pw_1 setzen, was beliebiges Einloggen ermöglicht. Ein Grund mehr, solche Abfragen mit der Benutzer-ID durchzuführen. (Siehe überübernächster Punkt.)

- t_pn: Die Spalten "empf" und "von" sollten nicht den Namen, sondern die ID der entsprechenden Nutzer enthalten. "datum" und "zeit" sollten zu einer Spalte vom Typ DATETIME zusammengefasst werden.

- t_termine: "user" sollte auf die ID zeigen, "tag", "monat", "jahr", "stunde", "minute" ergeben zusammengefasst wieder eine Spalte vom Typ DATETIME. (Du wirst angenehm überrascht sein, was du damit anstellen kannst, falls du irgendwann alle Daten ausgeben möchtest, die in einer Woche liegen oder so.)

- Analog dazu sollte auch in $_SESSION der Dreh- und Angelpunkt die User-ID sein, nicht der Name. (Grob dazu: Du prüfst bei der Registrierung nicht, ob ein Nutzername schon vergeben ist.)

- Du könntest im HTML-Markup versuchen, von Tabellen wegzukommen und nicht alles per echo auszugeben, um mehr Übersicht zu gewinnen. Siehe auch übernächster Punkt.

- Zudem könntest du vielleicht mal andenken, den Code in "Module" zu unterteilen (bietet sich hier an: PN und Termine und eventuell Optionen oder so). Etwa: ?module=pn&action=edit&id=9. Das ginge dann allerdings -- wie auch der nächste Punkt -- schon arg Richtung Objektorientierung, wenn man es ordentlich machen will.

- Aktuell sind die einzelnen Dateien noch nicht so umfangreich, dass Übersicht zu einem größeren Problem wird, aber generell ist es nie verkehrt, zusätzlich auf eine Trennung von "Logik" und "Ausgabe" abzuzielen. Das heißt, die Ermittlung/Verarbeitung der darzustellenden Daten in einer Datei zu erledigen, die Ausgabe in einer anderen. Der Vorteil davon ist, dass die "Darstellungsschicht" beliebig ausgetauscht werden kann und nicht so eng mit der Logik "verzahnt" ist. optionen.php ist vielleicht ein ganz gutes Beispiel, wo die Probleme mit der Übersicht so langsam anfangen.

Ich hoffe, du bist jetzt von den vielen Anmerkungen nicht völlig überrollt. :)

Gruß Marc
 
Danke marc! Ich werde an den ganzen Sachen arbeiten. Nur wie kann ich direkte Eingaben vom user verhindern? Das bei der Regsitrierung ist auch irgendwie logisch, dass man erst prüfen sollte :D Und dass vom Nutzernamen zur ID zu wechseln das wird hart, weil da muss ich im Grunde jede Datei doppelt überarbeiten, aber ich wills ja ordentlich haben. Danke, Marc (nochmal) für die Kritik :D (Kaum zu Glauben, dass es Menschen gibt, die sowas schreiben) Deswegen werde ich aufgrund von Wartungsarbeiten das Ding nochmal überarbeiten. Auch am Design lässt sich noch fummeln (Wenn ich Bock habe)...

Viele Grüße
Jonathan
 
Hab mal nur so ganz schnell über die index.php geschaut, werde das System gleich mal ausführlicher testen:


PHP: 
	switch ($aktion) {
		case "main":
		include "main.php";
		break;
		case "writetm":
		include "writetm.php";
		break;
		case "writepn":
		include "writepn.php";
		break;
		case "kalender":
		include "kalender.php";
		break;
		case "tm":
		include "tm.php";
		break;
		case "tmedit":
		include "tmedit.php";
		break;
		case "nachrichten":
		include "nachrichten.php";
		break;
		case "pn":
		include "pn.php";
		break;
		case "optionen":
		include "optionen.php";
		break;
		case "impressum":
		include "imgs/index.html";
		break;
		case "mitglieder":
		include "mitglieder.php";
		break;
		default:
		echo "Unbekannter Befehl";
		break;
	}
}

Da verstehe ich nicht ganz den Sinn.
Warum machst du's nicht direkt so:

PHP: 
include ($aktion.".php");

MfG Icy
 
Habe ich auch am Ende gedacht, und wenn einer die GET Angaben fälscht, gibt es dort keine Ausweichmöglichkeiten. Außerdem habe ich mit einem kleinen Teil angefangen zu programieren und auf einmal kam da so eine riesige Schlange raus, die aber dafür relativ sicher ist. Was ich immer für GET verwendet habe, dass waren immer nur unwichtige Sachen. Bei POST sieht es schon anders aus *schäm*... Kann mir denn einer verraten, wie ich direkte Benutzereingaben verhindern kann?

Viele Grüße
Jonathan
 
Verwende mysql_real_escape_string. Also z.B. so:
PHP: 
$id = mysql_real_escape_string($_GET['id']);
mysql_query("DELETE FROM list WHRE id = '$id'");
Das stellt eventuell vorkommenden Apostrophen (') und noch ein paar anderen Zeichen einen Backslash (\) voran. Dadurch merkt SQL, das hier noch nicht der Ende des Wertes kommt, sondern dass das wirklich ein Apostroph sein soll.
 
Danke Wolf! Kann ich das immer verwenden bei direkten Benutzereingaben (Auch wenn ich damit keine GETS sondern POSTS absichern muss)?
@eric: Du warst schon lange nicht mehr in ICQ on und außerdem würde ich mal ganz gerne über eines deiner Projekte unterhalten. Könntest du mir vielleicht ein paar Infos per PN zu deinem homieVZ schicken?

Viele Grüße
Jonathan
 
brainiac schrieb:
Kann ich das immer verwenden bei direkten Benutzereingaben (Auch wenn ich damit keine GETS sondern POSTS absichern muss)?
Zum Vergrößern anklicken....

Ja, so ist es für beides genau richtig.

brainiac schrieb:
Außerdem habe ich mit einem kleinen Teil angefangen zu programieren und auf einmal kam da so eine riesige Schlange raus, die aber dafür relativ sicher ist.
Zum Vergrößern anklicken....

Man könnte es verkürzen, aber je länger ich darüber nachdenke, desto besser gefällt mir die Lösung mit select-case für die Includes. Schlicht aus dem Grund, weil weitere Befehle in die Cases geschrieben werden können, die bei allen anderen Lösungen durch zusätzliche if-Konstrukte hinzugefügt werden müssten.

GET und POST:

The "get" method should be used when the form is idempotent (i.e., causes no side-effects). Many database searches have no visible side-effects and make ideal applications for the "get" method.

If the service associated with the processing of a form causes side effects (for example, if the form modifies a database or subscription to a service), the "post" method should be used.
Zum Vergrößern anklicken....

Requests, die Daten verändern (und nicht bloß darstellen), sollten immer per POST abgesetzt werden. Würde ich unter "guter Stil" abheften (und macht man wohl meist automatisch so), aber sollte man zumindest mal gehört haben.

brainiac schrieb:
Und dass vom Nutzernamen zur ID zu wechseln das wird hart, weil da muss ich im Grunde jede Datei doppelt überarbeiten, aber ich wills ja ordentlich haben.
Zum Vergrößern anklicken....

Ja, aber so wird's gemacht. Falls du demnächst übrigens irgendwann vor der Frage stehen solltest, wie du zu jeder User-ID in "von" in t_pn den passenden Usernamen ermittelst (weil du ihn im Posteingang darstellen willst), lies mal was über JOINs und widersteh der Versuchung, dafür eine zweite Query abzusetzen. (Ich sage das, weil ich wünschte, ich hätte das eher erfahren.)
 
brainiac schrieb:
Deswegen werde ich aufgrund von Wartungsarbeiten das Ding nochmal überarbeiten.
Zum Vergrößern anklicken....
Ich würd' alle deine Skripte überarbeiten. Die kranken alle an den selben Symptomenen. Du willst ja sicher nicht für Datenverlust o.ä. verantwortlich gemacht werden, von Leuten die deine Skripte verwenden.
 
Das ist aber noch anfällig auf XSS-Angriffe ;) Das heisst, wenn ich im Gesprächsraum HTML-Code eingebe, wird der so übernommen. Im besten Fall kann ich damit das Layout im Gesprächsraum kaputt machen. Im schlimmsten Fall kann ich mit Javascript den Angreifer auf eine ganz andere Seite umleiten und dort seine Eingaben speichern und ihn z.B. auffordern, sein Passwort einzugeben. Ich habe mal ein kleines Alert eingebaut ;)
Wenn du über alle Sachen, die vom Benutzer kommen und die du im HTML-Code ausgeben willst, die Funktion htmlspecialchars laufen lässt, sollte das Problem gelöst sein. Dadurch werden <, > und " in &lt; &gt; und &quot; umgewandelt.
PS: Du escapest zwar schön die Daten für SQL, aber du solltest bei der Ausgabe hier noch einmal die stripslashes-Funktion verwenden, damit die unschönen Backslashes für den Benutzer verschwinden, wenn er z.B. Anführungsstriche verwendet hat.
 
Du musst dich einloggen oder registrieren, um hier zu antworten.

Neueste Beiträge

Zurück
Oben