Datensätze ändern PHP

Ich programmiere für meine Schule ein Forum in php. Ich speichere Benutzernamen und Passwörter in einer Datenbank. Wenn man sich angemeldet hat soll man auch sein Passwort ändern können, so weit so gut da muss man dann das alte Passwort und das neue Eingeben, dann sollte der Datensatz in der Tabelle geändert werden, is aber nich.Hier der betreffende Abschnitt:


$pass = $_POST['Passwort'];
$passalt = $_POST['Passwortalt'];
mysql_query("UPDATE `Forum_Users` Set `Passwort` = ".$pass. " WHERE `Passwort` = ".$passalt);
echo("<a href = \"Forum_Startseite.php\">Zurück zur Startseite</a>");

Ausprobieren? Hier klicken:http://www.haferbriproductions.de/Forum/Forum_Startseite.php
Test Benutername:Test
Test Passwort: test

was passiert denn?

achja du solltest den Benutzernamen mit in die WHERE Klausel nehmen, weil sonst werden alle Passwörter geändert wenn alle das gleiche Passwort haben :wink:

ich habs probiert, aber irgendwie funtioniert die Übergabe des Namens an Pass_Ändern.php nicht.


<p style = \"text-align:right;\">
<form method=\"post\"action=\"Pass_ändern.php\">
<input type=\"submit\" name=\"annehmen\" value=\"Passwort ändern\">
<input type=\"hidden\" name=\"Benutzer\" value=\"".$name."\">
</form></p>

Urgs, du musst im Query natürlich auch mit übergeben, dass es sich um einen String handelt. Sonst wirst du immer einen SQL Fehler erhalten.


mysql_query("UPDATE `Forum_Users` Set `Passwort` =  '".$pass."' WHERE `Passwort` = '".$passalt."'");

Und damit es lesbarer wird würde ich dir die alternative Schreibweise ans Herz legen oder mit sprintf zu arbeiten


mysql_query("UPDATE `Forum_Users` Set `Passwort` =  '{$pass}' WHERE `Passwort` = '{$passalt}'");
// bzw
$queryString = "UPDATE `Forum_Users` Set `Passwort` =  '%s' WHERE `Passwort` = '%s'";
mysql_query( sprintf($queryString, $pass, $passalt));

Des weiteren empfiehlt es sich auf eine Lib zurück zu greifen die einem die Queries absichert. Hier fehlt zb. noch ein mysql_real_escape_string. Denn magic_quote_gpc wird auf den meisten System entweder inaktiv sein bzw. gar nicht mehr existieren :wink:

Sorry, aber wenn ich das schon sehe. Wo fang ich nur an?

  1. Die MySQL-Extension ist veraltet, langsam und sollte auf keinen Fall mehr benutzt werden. Dazu wurde PDO erfunden.
  2. Bitte keine direkten Queries mit rein-konkatenierten Paramtern - Prepared Statements sind sauberer, sicherer und schützen vor gefährlicher SQL-Injection.
  3. Unverschlüsselte Passwörter! Ein absolutes No-Go. Sofern (hoffentlich) PHP 5.5 eingesetzt wird, gibt es mit password_hash() und password_verify() simple Möglichkeiten, um Passwörter sicher und einfach zu verschlüsseln. Für ältere PHP-Versionen gibt es immer noch crypt(). Aber unverschlüsselt sollte man Passwörter absolut niemals und unter garkeinen Umständen speichern.
  4. Das Passwort als einzige Bedingung für den SQL-Query. Was passiert, wenn mehrere Benutzer das gleiche (ungesalzene) Passwort haben? Die Benutzer-ID ist als weitere Bedingung zwingend notwendig.
  5. echo im Source. Bei kleinen, schnellen Scripts okay, aber wenn das eine Forensoftware werden soll, wäre ein Templatesystem sinnvoll oder wenigstens eine Ausgabe außerhalb der <?php ?>-Tags, viele einzelne echos sind in PHP relativ langsam.
  6. Warum die POST-Werte in extra Variablen kopieren? Kostet nur wertvolle Rechenzeit und ist absolut sinnlos.

Und noch @Evil_Devil :
Wie gesagt, MySQL-Extension ist veraltet, ebenso sollte man statt dem potenziell unsicheren mysql_real_escape_string()-Quatsch Prepared Statements verwenden. Weiterhin ist Magic Quotes alles andere als die richtige Methode, um vor SQL-Injection zu schützen. Nicht umsonst wurde es schon lange abgeschafft.

Ebenso finde ich die Lösung mit sprintf() schwachsinnig. Die verschiedenen Konkatenationstypen in PHP sind dazu da, dass man sie benutzt. Im Gegensatz zu sprintf(), welches bereits eine komplette Funktion ist, stellen diese nur spezielle Opcodes dar und sind dadurch bereits wesentlich schneller. Funktionsaufrufe in PHP sind allg. relativ langsam und sprintf() für so einen Zweck zu verwenden ist einfach daneben. Ob man jetzt den Konkatenationsoperator (.) hernimmt oder die Variablen “direkt” einfügt macht performance-technisch keinen wirklichen Unterschied und ist dem Geschmack des Entwicklers überlassen.

[QUOTE=pp3345]
Und noch @Evil_Devil :
Wie gesagt, MySQL-Extension ist veraltet, ebenso sollte man statt dem potenziell unsicheren mysql_real_escape_string()-Quatsch Prepared Statements verwenden. Weiterhin ist Magic Quotes alles andere als die richtige Methode, um vor SQL-Injection zu schützen. Nicht umsonst wurde es schon lange abgeschafft.[/QUOTE]
Prepared Statements sind so lange gut wie das Framework sie entsprechend unterstützt … hab da schon diverse Konstellationen erlebt in denen ein Prepared Statement zum einen langsamer war und zum anderen dank PDO bei speziellen Operationen gerne mal failed.

Nachtrag: magic_quotes_gpc wird seit php 5.4 eh nicht mehr unterstützt, von daher kann man es eh nicht mehr verwenden :smiley:

Magic Quotes waren bereits in PHP 5.3 deprecated und wurden mit PHP 5.4 zum Glück endlich abgeschafft. Wenn das ein Framework ein akutes Problem mit Prepared Statements hat und keinen ähnlich sicheren Query Builder bietet, würde ich eher das Framework wechseln.

Naja … ZF2 ist schon in Ordnung … in den Fällen waren es immer primär die Datenmengen … alles im Sechsstelligen Bereich und aufwärts ist grundsätzlich nicht mehr so einfach zu handhaben :wink:

Für eine richtige MVC-Trennung, würde ich bei einer Forensoftware auf Smarty setzen.
Für die Datenbank-Verbindung eignet sich PDO sehr gut. Wenigstens würde ich aber mysqli verwenden. Ansonsten schließe ich mich meinen Vorrednern bezüglich prepared-Statements an!

Schmeiß Smarty weg und nutze was schnelleres oder direkt die alternative Syntax für IF und Schleifen die einem PHP zur Verfügung stellt. Smarty wird bei größeren Projekten schnell zu einer Bremse. Ist jedenfalls meine Erfahrung.

Smarty ist soo eine Bremse? Ich dachte, dass hätten die in den Griff bekommen. Ich habe ehrlich gesagt Smarty noch nie verwendet, wollte das aber mal verwenden…

Sollte ich wohl lieber lassen. :slight_smile:

[OFFTOPIC]
Bist du in der PHP-Programmierung beschäftigt?
[/OFFTOPIC]

Was ist denn schneller kennst du eine Template-Engine?

[QUOTE=jamesv1994][OFFTOPIC]
Bist du in der PHP-Programmierung beschäftigt?
[/OFFTOPIC][/QUOTE]
… Ja

@Template Engine: Ich würde aktuell gar keine mehr verwenden. Besser einen Cache nutzen, das ist viel mehr Zeitgewinn. Eine Template Engine brauch es eigentlich nur, wenn man Designer hat die an den Templates arbeiten sollen und die nichts von PHP & Co verstehen. Aber selbst dann müssen die erstmal die Template Sprache lernen …

  1. Die MySQL-Extension ist veraltet, langsam und sollte auf keinen Fall mehr benutzt werden. Dazu wurde PDO erfunden.

Schon recht. Habe es mir auch angewöhnt.
Wobei ich gestehen muss, mysql_* zeugs bei kleinen schnellen Dingen (die eh nur für kurze Zeit benötigt werden) trotzdem nutze.

  1. Bitte keine direkten Queries mit rein-konkatenierten Paramtern - Prepared Statements sind sauberer, sicherer und schützen vor gefährlicher SQL-Injection.

Bzw. Filtern lassen. Bei Prepared Statements wäre dies Thema erledigt, richtig.

  1. Unverschlüsselte Passwörter! Ein absolutes No-Go. Sofern (hoffentlich) PHP 5.5 eingesetzt wird, gibt es mit password_hash() und password_verify() simple Möglichkeiten, um Passwörter sicher und einfach zu verschlüsseln. Für ältere PHP-Versionen gibt es immer noch crypt(). Aber unverschlüsselt sollte man Passwörter absolut niemals und unter garkeinen Umständen speichern.

Nicht nur. Gibt ja auch MD5 oder was auch immer. Zudem sollte man dies mit Salt’s verbinden damit man Bruteforce Attacken entgegen wirken kann.

  1. echo im Source. Bei kleinen, schnellen Scripts okay, aber wenn das eine Forensoftware werden soll, wäre ein Templatesystem sinnvoll oder wenigstens eine Ausgabe außerhalb der <?php ?>-Tags, viele einzelne echos sind in PHP relativ langsam.

Ist mir neu. Kannst du das erläutern?

Gut, dass man nicht unbedingt in jedem Wort reinen Text ausgibt klingt logisch. Was macht man aber bei Systemen die Beispielsweise i18n-Like translaten?
Beispielsweise über Gettext? Da werden auch Wörter oder ganze Sätze durch Gettext-Funktionen gejagt und dann ausgegeben. Ergo WordPress & Co machen es nicht anders, wenn man da die echo/print Ausgaben zählt kommt man garantiert auf einer sehr hohe summe.

Zudem sollte man echo immer dem print vorziehen sofern möglich.

Wegen printf bzw. sprintf:
Ich nutze es für so einige Dinge auch. Wobei man auch hier ggf. Sicherheitstechnische Filter einbauen sollte. Hatte irgendwo mal gelesen dass man mittels Byte-Offsets diese funktionen injecten kann. Finde des aber grad nicht mehr.

[QUOTE=Evil_Devil;22888]

@Template Engine: Ich würde aktuell gar keine mehr verwenden. Besser einen Cache nutzen, das ist viel mehr Zeitgewinn.[/QUOTE]

Das stimmt auch wieder :slight_smile:

Schon recht. Habe es mir auch angewöhnt.
Wobei ich gestehen muss, mysql_* zeugs bei kleinen schnellen Dingen (die eh nur für kurze Zeit benötigt werden) trotzdem nutze.

Dann aber bitte mysqli_*
:slight_smile:

Sorry, wenn ich das so sage, aber eure Diskussion bringt mich erlichgesagt kein Stück weiter.

Bitte tu dir einen gefallen und nimm eine der vielen fertigen Forensoftware.

Ich komme nichtmal auf die Passwort-Ändern-Seite. Da sind unendlich viele Programmierfehler drin. Es fängt mit dem fehlerhaften Encoding der Seite an, geht darüber, dass die Verlinkung der Seiten falsch ist (Groß- und Kleinschreibung beachten!) und von den ganzen Sicherheitslücken wollen wir gar nicht erst anfangen…

Dein Login/Registrier-Modul ist fehlerhaft und ziemlich naiv :slight_smile:
Naja ist ja egal. Also erstmal schau dir dein Encoding an. Dann
ich habe mich zweimal mit Username=te und PW=te registriert. Wenn ich versuche mich einzuloggen:
Login fehlerhaft, obwohl die Registrierung angeblich erfolgreich war. Wenn du in der DB mit einem Hash arbeitest, musst du auch beachten, das eingebene PW zu hashen, bevor du es vergleichst :smiley:

Jop, das war´s auch erstmal. Bei Fragen, kannste dich ja mal melden.