PHP: Bedingung wird nie erfüllt

[Yadgar]

Hi(gh)!

Vorab zum Kontext:

Meine Datenbank soll das Stichwortverzeichnis für ein mehrere Monate umspannendes privates Tagebuch verwalten. Sie enthält die Stammdatentabellen STICHWOERTER (nr, stichwort) und KATEGORIE (nr, kategorie) sowie die Relationstabellen STICHWORT_DATUM (nr, stichwoerter_nr, datum) und STICHWORT_KATEGORIE (nr, stichwoerter_nr, kategorie_nr). Indizes sind jeweils PRIMARY über nr und UNIQUE über die nachfolgenden Felder.

Die Grundfunktionalität der mit PHP programmierten Eingabemasken ist mittlerweile fertiggestellt, mir geht es jetzt die Möglichkeit nachträglicher Bearbeitung von bereits eingegebenen Datensätzen.

Im oberen Teil des Eingabebereiches werden die Datensätze zum "Durchblättern" als Tabelle in Zehnerblöcken angezeigt, die erste Spalte enthält jeweils interne Links ($_GET-Methode) zum Bearbeiten und Löschen.

Klickt man diese Links an, wird die Indexnummer des jeweiligen Datensatzes an die neu aufgerufene Eingabeseite übermittelt. In diesem Fall sollen die bisherigen Werte als Voreinstellung in den Eingabeelementen des Formulars angezeigt werden.

Bei einfachen Textfeldern (in meinem konkreten Fall: datum in STICHWORT_DATUM) funktioniert das einwandfrei, während ich es bei den Auswahlmenüs (<select><option>...) aus mir unerfindlichen Gründen nicht hinbekomme.

Damit im Auswahlmenü das der übernommenden Indexzahl aus STICHWORT (also STICHWORT_DATUM.stichwoerter_nr) entsprechende Stichwort im Klartext anzeigt wird, schicke ich folgende Query ab:

  $sql3 = "SELECT STICHWOERTER.stichwort FROM STICHWOERTER, STICHWORT_DATUM WHERE STICHWORT_DATUM.stichwoerter_nr = STICHWOERTER.nr AND STICHWORT_DATUM.nr = '".$req."'";

$req nimmt hier den Wert von $_GET["edit"] auf.

Der komplette relevante Code sieht so aus:

$sql1 = "SELECT nr, datum FROM STICHWORT_DATUM";
$res1 = $db->query($sql1);
qcheck($res1, $db);
if (empty($_GET["edit"]))
{
  $entry1 = "";
  $entry2 = "";
}
else // falls Datensatz bearbeitet werden soll
{
  while($zeile1 = $res1->fetch_array()) 
  {
    if ($zeile1[0] == $_GET["edit"])
    {
       $entry2 = $zeile1[1]; // vorselektierter Eintrag für Datum
      break;
    }
  }
  $req = $_GET["edit"];
  $sql3 = "SELECT STICHWOERTER.stichwort FROM STICHWOERTER, STICHWORT_DATUM WHERE STICHWORT_DATUM.stichwoerter_nr = STICHWOERTER.nr AND STICHWORT_DATUM.nr = '".
  // 
  $req."'";
  $res3 = $db->query($sql3);
  qcheck($res3, $db);
  $entry1 = $res3->fetch_array(); // Eintrag für Stichwort im Auswahlmenü
}

$sql2 = "SELECT stichwort FROM STICHWOERTER";
$res2 = $db->query($sql2);
qcheck($res2, $db);

var_dump($entry1); // Kontrollausgabe

?>


      
<form action="<?php echo htmlspecialchars($_SERVER["PHP_SELF"]); ?>" method="POST">
	Stichwort:<br>
  <select name="stichwort">
    <?php
      $zusatz="";
      while ($zeile2 = $res2->fetch_array())
      {
        if (!empty($_GET["edit"]) && $zeile2[0] == $entry1)
        {
          echo "Bedingung erfüllt!<br>"; // Kontrollausgabe
          $zusatz = " selected";
        }
        echo "<option".$zusatz.">".$zeile2[0]."</option>\n";
      }
    ?>
  </select><br>
  Datum<br>

  <?php
    echo '<input type="text" name="datum" size="10" value="'.$entry2.'">';
  ?>

Die Kontrollausgabe von $entry1 zeigt den korrekten String... aber trotzdem wird die Bedingung (!empty($_GET["edit"]) && $zeile2[0] == $entry1) nie erfüllt! Warum?

Bis bald im Khyberspace!

Yadgar

[Rolf B]

Hallo Yadgar,

  $req = $_GET["edit"];
  $sql3 = "SELECT ... WHERE ... AND STICHWORT_DATUM.nr = '".
  // 
  $req."'";

Schönen Gruß von Bobby Tables. Bestimmt nicht zum ersten Mal!

Im Übrigen wird das Problem wohl an der Ursache liegen, die man dem Code aber nicht ansieht. Ich weiß nicht, was Du da aus der Stichwörter-Tabelle herausliest. Das Problem könnten Spaces sein, die in der DB stehen. Z.B. weil es keine VARCHAR-Spalte ist. Das kriegst Du raus, wenn Du die "Kontrollausgabe" vor den if ziehst und die beiden Werte, die Du vergleichst, ausgibst. Und dann am besten in einem Monospace-Font anschaust (sprich: In der Sourcecode-Ansicht der Entwicklerwerkzeuge), und nicht das, was der Browser draus macht.

Rolf

[Der Martin]

Hallo,

Das kriegst Du raus, wenn Du die "Kontrollausgab" vor den if ziehst und die beiden Werte, die Du vergleichst, ausgibst.

und zwar mit einem bewusst gesetzten Begrenzer-Zeichen vor und nach dem String, um auch abschließende Leerzeichen zu sehen. Oder als Hex-Dump, dann ist wirklich jeder noch so subtile Unterschied sichtbar.

Live long and pros healthy,
 Martin

[dedlfix]

Tach!

Das kriegst Du raus, wenn Du die "Kontrollausgab" vor den if ziehst und die beiden Werte, die Du vergleichst, ausgibst.

und zwar mit einem bewusst gesetzten Begrenzer-Zeichen vor und nach dem String, um auch abschließende Leerzeichen zu sehen.

Also ganz einfach mit var_dump().

dedlfix.

[Der Martin]

Hallo,

und zwar mit einem bewusst gesetzten Begrenzer-Zeichen vor und nach dem String, um auch abschließende Leerzeichen zu sehen.

Also ganz einfach mit var_dump().

wo er recht hat ... 😉

Live long and pros healthy,
 Martin

[Robert B.]

Moin Yadgar,

Damit im Auswahlmenü das der übernommenden Indexzahl aus STICHWORT (also STICHWORT_DATUM.stichwoerter_nr) entsprechende Stichwort im Klartext anzeigt wird, schicke ich folgende Query ab:

$sql3 = "SELECT STICHWOERTER.stichwort FROM STICHWOERTER, STICHWORT_DATUM WHERE STICHWORT_DATUM.stichwoerter_nr = STICHWOERTER.nr AND STICHWORT_DATUM.nr = '".$req."'";

$req nimmt hier den Wert von $_GET["edit"] auf.

Das ist so keine gute Idee, weil das eine astreine SQL-Injection-Lücke ist. Man braucht dein Script nur mit dem URL-Parameter ?edit=' or ''=' aufrufen und es wird einfach mal alles SELECTiert. Du möchtest Escaping oder besser prepared statements verwenden!

Der komplette relevante Code sieht so aus:

<?php
// …

  while($zeile1 = $res1->fetch_array()) 
  {
    if ($zeile1[0] == $_GET["edit"])
    {
       $entry2 = $zeile1[1]; // vorselektierter Eintrag für Datum
      break;
    }
  }

// …

  $entry1 = $res3->fetch_array(); // Eintrag für Stichwort im Auswahlmenü
}

// …

?>

<form action="<?php echo htmlspecialchars($_SERVER["PHP_SELF"]); ?>" method="POST">
	Stichwort:<br>

Das Stichwort möchte wohl ein label sein.

  <select name="stichwort">
    <?php
      $zusatz="";
      while ($zeile2 = $res2->fetch_array())
      {
        if (!empty($_GET["edit"]) && $zeile2[0] == $entry1)
        {
          echo "Bedingung erfüllt!<br>"; // Kontrollausgabe
          $zusatz = " selected";
        }
        echo "<option".$zusatz.">".$zeile2[0]."</option>\n";
      }
    ?>
  </select><br>

• Wenn die Bedingung einmal erfüllt ist, wird $zusatz gesetzt, es wird aber nie zurückgesetzt, falls die Bedingung nicht mehr erfüllt ist.
• $zeile2[0] ist vermutlich ein Skalar, $entry1 ist ein Array.
• $zeile2[0] wird unbehandelt in den HTML-Kontext gesetzt. Damit besteht die Gefahr von Cross Site Scripting.

⇒ Du möchtest dich mit dem Thema Kontextwechsel beschäftigen.

  Datum<br>

  <?php
    echo '<input type="text" name="datum" size="10" value="'.$entry2.'">';
  ?>

$entry2 wird unbehandelt in den HTML-Kontext gesetzt. Damit besteht die Gefahr von Cross Site Scripting.

Die Kontrollausgabe von $entry1 zeigt den korrekten String... aber trotzdem wird die Bedingung (!empty($_GET["edit"]) && $zeile2[0] == $entry1) nie erfüllt! Warum?

Vermutlich ist der Vergleich eines Skalars mit einem Array false.

Viele Grüße
Robert

[Rolf B]

Hallo Robert,

• Wenn die Bedingung einmal erfüllt ist, wird $zusatz gesetzt, es wird aber nie zurückgesetzt, falls die Bedingung nicht mehr erfüllt ist.

Good catch. Der Init muss in den while. Und könnte da zu einem ?: Operator kollabieren.

• $zeile2[0] ist vermutlich ein Skalar, $entry1 ist ein Array.

Au weia. Natürlich. Es kommt aus fetch_array.

• $zeile2[0] wird unbehandelt in den HTML-Kontext gesetzt. Damit besteht die Gefahr von Cross Site Scripting.

Die Forderung ist richtig, aber die Begründung kann variieren. Ob XSS-Gefahr besteht, hängt davon ab, wer die Stichworttabelle pflegt. Muss man seinem eigenen Tabellen-Admin misstrauen? Das Hauptargument ist: Es sind Daten aus einer Tabelle, die da ausgegeben werden, es könnten zum Kontext unpassende Zeichen drinstehen, also ab damit nach htmlspecialchars. Auf diese Funktion verzichtet man nur dann, wenn man weiß, dass in der Variablen HTML steht, das man als HTML ausgeben will.

Rolf

[Yadgar]

Hi(gh)!

Die Forderung ist richtig, aber die Begründung kann variieren. Ob XSS-Gefahr besteht, hängt davon ab, wer die Stichworttabelle pflegt. Muss man seinem eigenen Tabellen-Admin misstrauen?

Wohl kaum, der Tabellen-Admin bin nämlich ich selbst und niemand sonst. Und daran wird sich auch nichts ändern, da sich diese Datenbank ausschließlich auf ein streng persönliches Tagebuch bezieht. Dieses Tagebuch ist über weite Strecken ein Produkt meines kranken Geistes und würde öffentlich gemacht ganze Bündel von Straftatbeständen erfüllen - zur eurer Beruhigung: Kinderporno ist nicht dabei - also bleibt es lokal auf meiner Festplatte. Allenfalls mein zukünftiger Psychotherapeut würde es zu lesen bekommen, hätte aber ebenfalls keinen schreibenden Zugriff auf die Datenbank.

Bis bald im Khyberspace!

Yadgar

[Robert B.]

Moin Yadgar,

Die Forderung ist richtig, aber die Begründung kann variieren. Ob XSS-Gefahr besteht, hängt davon ab, wer die Stichworttabelle pflegt. Muss man seinem eigenen Tabellen-Admin misstrauen?

Wohl kaum, der Tabellen-Admin bin nämlich ich selbst und niemand sonst. Und daran wird sich auch nichts ändern, da sich diese Datenbank ausschließlich auf ein streng persönliches Tagebuch bezieht. Dieses Tagebuch ist über weite Strecken ein Produkt meines kranken Geistes […]

Nichtsdestrotrotz könn(t)en deine Schlagworte (irgendwann) so „unverdächtige“ Zeichen wie "Anführungszeichen', das „Kaufmanns-Und“ & oder >spitze Klammern< enthalten. Das ist zwar dann nicht zwingend eine Sicherheitslücke, kann dir aber die Anzeige zerschießen.

Im Übrigen wird im Allgemeinen mehr Zeit und Energie darauf verwandt zu argumentieren, warum ein Kontextwechsel scheinbar harmlos ist, anstatt die korrekte Behandlung einfach schnell einzubauen. Und nichts hält sich hartnäckiger als die Wiederverwendung von Code und damit auch von solchen „Schlampereien“ bzw. Anti-Patterns. Es richtig (TM) zu machen, zahlt sich immer aus.

Viele Grüße
Robert

[Der Martin]

Hallo miteinander,

Und nichts hält sich hartnäckiger als die Wiederverwendung von Code und damit auch von solchen „Schlampereien“ bzw. Anti-Patterns.

genau, nichts hält länger als ein Provisorium.

Live long and pros healthy,
 Martin

[Auge]

Hallo

Im Übrigen wird im Allgemeinen mehr Zeit und Energie darauf verwandt zu argumentieren, warum ein Kontextwechsel scheinbar harmlos ist, anstatt die korrekte Behandlung einfach schnell einzubauen. Und nichts hält sich hartnäckiger als die Wiederverwendung von Code und damit auch von solchen „Schlampereien“ bzw. Anti-Patterns. Es richtig (TM) zu machen, zahlt sich immer aus.

Bugreport zur Forensoftware: Man kann einen Beitrag nur einmal bewerten. 😉

Der Beitrag meines Vorredners … ähh … schreibers hätte es allein nur wegen des hier zitierten Absatzes verdient, dass jeder, der hier Bewertungen abgeben kann, dies auch minimal zehnmal in Form einer positiven Bewertung tut. Das aber nicht, wie es jetzt implementiert ist, indem der Status zwischen einer positiven und einer Nichtbewertung wechselt, sondern so, dass sich die Klicks aufsummieren.

Ansonsten, was Martin sagt.

Tschö, Auge

[Robert B.]

Moin Auge,

Im Übrigen wird im Allgemeinen mehr Zeit und Energie darauf verwandt zu argumentieren, warum ein Kontextwechsel scheinbar harmlos ist, anstatt die korrekte Behandlung einfach schnell einzubauen. Und nichts hält sich hartnäckiger als die Wiederverwendung von Code und damit auch von solchen „Schlampereien“ bzw. Anti-Patterns. Es richtig (TM) zu machen, zahlt sich immer aus.

Bugreport zur Forensoftware: Man kann einen Beitrag nur einmal bewerten. 😉

also ich kann Beiträge einmal positiv/negativ und einmal als interessant bewerten 😜

Der Beitrag meines Vorredners … ähh … schreibers hätte es allein nur wegen des hier zitierten Absatzes verdient, dass jeder, der hier Bewertungen abgeben kann, dies auch minimal zehnmal in Form einer positiven Bewertung tut.

Vielen Dank für die Blumen! Aber ich schreibe das doch wegen der Message und nicht der Bewertungen 😉

Viele Grüße
Robert

[Auge]

Hallo

Der Beitrag meines Vorredners … ähh … schreibers hätte es allein nur wegen des hier zitierten Absatzes verdient, dass jeder, der hier Bewertungen abgeben kann, dies auch minimal zehnmal in Form einer positiven Bewertung tut.

Vielen Dank für die Blumen! Aber ich schreibe das doch wegen der Message und nicht der Bewertungen 😉

… und diese Message halte ich für überaus wichtig. 👍

Tschö, Auge

[Yadgar]

Hi(gh)!

Moin Yadgar,

Das ist so keine gute Idee, weil das eine astreine SQL-Injection-Lücke ist. Man braucht dein Script nur mit dem URL-Parameter ?edit=' or ''=' aufrufen und es wird einfach mal alles SELECTiert. Du möchtest Escaping oder besser prepared statements verwenden!

Nein, das möchte ich in diesem Fall nicht, denn: https://forum.selfhtml.org/self/2021/oct/14/php-bedingung-wird-nie-erfullt/1792511#m1792511

Das Stichwort möchte wohl ein label sein.

Ja, das macht man heutzutage wohl so...

• Wenn die Bedingung einmal erfüllt ist, wird $zusatz gesetzt, es wird aber nie zurückgesetzt, falls die Bedingung nicht mehr erfüllt ist.

Stimmt, das ist ein Fehler!

• $zeile2[0] ist vermutlich ein Skalar, $entry1 ist ein Array.

Yep!

• $zeile2[0] wird unbehandelt in den HTML-Kontext gesetzt. Damit besteht die Gefahr von Cross Site Scripting.

s. o.

$entry2 wird unbehandelt in den HTML-Kontext gesetzt. Damit besteht die Gefahr von Cross Site Scripting.

s. o.

Vermutlich ist der Vergleich eines Skalars mit einem Array false.

Ist er, ich hatte nicht bedacht, dass fetch_array() auch im Falle eines einzigen Feldeintrags als Ergebnis dieses immer in doppelter Ausführung (mit numerischem und Label-Index) und damit natürlich als Array raushaut...

Also korrigierte ich den Code:

  <select name="stichwort">
    <?php
      
      while ($zeile2 = $res2->fetch_array())
      {
        echo $zeile2[0]."<br>";
        if (!empty($_GET["edit"]) && $zeile2[0] == $entry1[0]) // jetzt mit Index!
        {
          echo "Bedingung erfüllt!<br>"; // Kontrollausgabe
          $zusatz = " selected";
        }
        else
        {
          $zusatz =""; // auf Leerstring zurückgesetzt falls Bedingung unwahr
        }
        echo "<option".$zusatz.">".$zeile2[0]."</option>\n";
      }
    ?>
  </select><br>

...und jetzt funktioniert es! Danke für die Hinweise!

Bis bald im Khyberspace!

Yadgar

[Robert B.]

Moin Yadgar,

Das ist so keine gute Idee, weil das eine astreine SQL-Injection-Lücke ist. Man braucht dein Script nur mit dem URL-Parameter ?edit=' or ''=' aufrufen und es wird einfach mal alles SELECTiert. Du möchtest Escaping oder besser prepared statements verwenden!

Nein, das möchte ich in diesem Fall nicht, denn: https://forum.selfhtml.org/self/2021/oct/14/php-bedingung-wird-nie-erfullt/1792511#m1792511

Mit spitzen Klammern machst du hier im Forum übrigens aus einer URL einen Link.

Ich habe dir dort geantwortet, warum du das trotzdem tun möchtest.

Das Stichwort möchte wohl ein label sein.

Ja, das macht man heutzutage wohl so...

Es hat auch gewisse Vorteile, weil das label nämlich auch klickbar ist. Bei einem Radiobutton oder eine Checkbox macht sich das richtig bemerkbar – oder wenn es ums Styling geht.

• $zeile2[0] wird unbehandelt in den HTML-Kontext gesetzt. Damit besteht die Gefahr von Cross Site Scripting.

s. o.

siehe „seitwärts“

$entry2 wird unbehandelt in den HTML-Kontext gesetzt. Damit besteht die Gefahr von Cross Site Scripting.

s. o.

siehe „seitwärts“

Vermutlich ist der Vergleich eines Skalars mit einem Array false.

Ist er, ich hatte nicht bedacht, dass fetch_array() auch im Falle eines einzigen Feldeintrags als Ergebnis dieses immer in doppelter Ausführung (mit numerischem und Label-Index) und damit natürlich als Array raushaut...

Es gibt laut Handbuch von fetch_array() genau einen Fall, in dem kein Array zurückgegeben wird:

Return Values

Returns an array of values that corresponds to the fetched row or null if there are no more rows in result set.

Und mir ist bislang noch kein Fall untergekommen, in dem das PHP-Handbuch eine Funktion falsch beschrieben hätte.

Also korrigierte ich den Code:

… in Teilen …

  <select name="stichwort">
    <?php
      
      while ($zeile2 = $res2->fetch_array())
      {
        echo $zeile2[0]."<br>";
        if (!empty($_GET["edit"]) && $zeile2[0] == $entry1[0]) // jetzt mit Index!
        {
          echo "Bedingung erfüllt!<br>"; // Kontrollausgabe
          $zusatz = " selected";
        }
        else
        {
          $zusatz =""; // auf Leerstring zurückgesetzt falls Bedingung unwahr
        }
        echo "<option".$zusatz.">".$zeile2[0]."</option>\n";
      }
    ?>
  </select><br>

...und jetzt funktioniert es!

… bis …

Viele Grüße
Robert

[Rolf B]

Hallo Robert,

Es hat auch gewisse Vorteile, weil das label nämlich auch klickbar ist.

Was dem optischen Eindruck nach stimmt, aber nicht ganz präzise ist. Labels klickbar zu nennen würde beinhalten, dass ein click-Handler für sie sinnvoll sein könnte. Das wäre ein Irrtum. Labels an sich sind nicht interaktiv.

Aber sie transferieren den Klick an das gelabelte Element, sofern es ein Form Control ist. Wenn ich ein div labele, passiert das nicht. Bei einem summary/details-Element auch nicht.

<label id="lfoo" for="foo">Sag was: </label>
<input id="foo" type="text">

Script:

document.body.addEventListener("click", function(event) {
   console.log("click auf " + event.target.id);
})

gibt bei Klick auf das Label aus:

click auf lfoo
click auf foo

und der Fokus wandert in die Textbox.

Rolf

[Rolf B]

Hallo Yadgar,

Du möchtest Escaping (...) verwenden!

Nein, das möchte ich in diesem Fall nicht

Es geht nicht um "ich möchte". Es geht um "wie macht man es richtig". Da sollte man tatsächlich nicht abwägen, ob das nötig ist oder nicht. Diese Abwägung dauert länger als der Einbau der Maskierung.

Ein htmlspecialchars frisst nur einen Krümel, kein Brot. Gewöhne es Dir einfach an, bei allen Ausgaben an den Browser. Sonst fällt es Dir irgendwann auf die Füße, mit einer spitzen Kante voraus.

Rolf

[Robert B.]

Moin Rolf,

Ein htmlspecialchars frisst nur einen Krümel, kein Brot. Gewöhne es Dir einfach an, bei allen Ausgaben an den Browser. Sonst fällt es Dir irgendwann auf die Füße, mit einer spitzen Kante voraus.

… und zwar sprichwörtlich.

Viele Grüße
Robert

[TS]

Hello,

Du möchtest Escaping (...) verwenden!

Nein, das möchte ich in diesem Fall nicht

Es geht nicht um "ich möchte". Es geht um "wie macht man es richtig". Da sollte man tatsächlich nicht abwägen, ob das nötig ist oder nicht. Diese Abwägung dauert länger als der Einbau der Maskierung.

Es geht an dieser Stelle auch um Verantwortung. Verantwortung für das ganze Netz. Denn eine SQL-Injection-Lücke kann durchaus dazu führen, dass der Host gekapert wird und dann für ROBOT-ANGRIFFE auf andere Hosts bereitgehalten wird.

Glück Auf
Tom vom Berg

[Rolf B]

Hallo Tom,

du hast Recht, dass SQL Injection Folgeschäden haben kann. Deswegen ja mein Gruß von Bobby Tables (vor allem von seiner Mama!) gleich zu Anfang.

Es ging just nun aber um HTML Maskierung 😉. Ich glaube, damit verpestet man keinen Server. Nur den Client. Bis der dann die Pest woandershin weiterreicht…

Rolf

[Robert B.]

Moin Rolf,

du hast Recht, dass SQL Injection Folgeschäden haben kann. Deswegen ja mein Gruß von Bobby Tables (vor allem von seiner Mama!) gleich zu Anfang.

Es ging just nun aber um HTML Maskierung 😉.

Nein, es ging auch um SQL-Injection.

Viele Grüße Robert