$_get und schreibweise...?

[sven k.]

sorry das der threadtitel ein wenig verwirrend ist, hab aber keinen plan wie ich das problem im betreff beschreiben soll.

hier mal der code:

<?php
$doc = DOMDocument::load("test.xml");
$id = $_GET['id'];
$xp = new domxpath($doc);
$titles = $xp->query('DVD[CollectionNumber="$id"]');


foreach ($titles as $node) {
 foreach($node->childNodes as $cnode) {
  if($cnode->nodeName == Title) {
   print $cnode->nodeName .": ". $cnode->textContent . "\n";
  }
 }
}


?>

es geht also darum das ich diese id bekomme (also eine zahl), diese in $id schreibe und dann das eben auswerten will.

$titles = $xp->query('DVD[CollectionNumber="$id"]'); 

bekomme aber damit keine ausgabe bzw. wenn ich das "=$id" schreibe dann gibts ne fehlermeldung.



was mache ich da falsch?

[marc75]

//mal so versucht?
$titles = $xp->query('DVD[CollectionNumber='.$id.']'); 

[DSB]

Der Unterschied bei ' und " ist folgender.
Beim Umschließen eines Strings mit Gänsefüßchen (") wird der Inhalt des Strings noch einmal geparsed und Variablen werden durch ihren Wert ersetzt.
Also wird aus: "hallo, ich heisse $name !" bei der Ausgabe "hallo, ich heisse Daniel ." (wenn $name den Wert "Daniel" hat)

Bei Hochkommas (') wird nicht mehr geparsed und deshalb sieht die Ausgabe dann genauso aus, wie vorher: aus 'hallo, ich heisse $name !' wird eben 'hallo, ich heisse $name !'

Genau das passiert bei der Übergabe Deines Queries. Die Variable $id wird nicht durch ihren Wert ersetzt, weil Du den String in Hochkommas gesetzt hast. Aus diesem Grund sollte man Varibalen auch immer außerhalb von Strings platzieren. Genauso wie Marc es gemacht hat.
So sieht man auf einen Blick, dass es sich um eine Variable handelt und das oben erklärte Problem kann nicht auftreten.

[ghostgambler]

Re: $_get und schreibweise...?

$id = $_GET['id']; 

Wozu schreibst du eine Variable in eine andere Variable???
Damit man später suchen darf, woher die Variable gekommen ist?
Außerdem solltest du noch ne Überprüfung einbauen, ob die id z.B. eine Zahl ist (natürlich nicht wie hier, auf eine Zeile gedrängt, sondern mit eckigen Klammern drum usw., damit es leserlich ist!):

if (!is_numeric($_GET['id'])) die('Die id ist keine Zahl!'); 

Außerdem wäre dir der Fehler wohl nicht unterlaufen, wenn du dir das, was du an $xp->query übergibst vorher nochmal ausgeben gelassen hättest...z.B.:

$query = 'DVD[CollectionNumber="$id"]';
echo htmlentities($query);
$titles = $xp->query($query); 

Und du hättest den Fehler sofort gesehen und diese Veränderung am Code (welche nicht von Dauer sein muss!), solltest du immer, zuerst machen, da erspart man sich ne Menge sonstige Sucherei mit...

[penizillin]

damit sollte man vorsichtig umgehen.
was gibt

is_numeric('5.5');
//und
is_numeric('.5'); 

zurück? in wiefern ist es erwünscht?

[ghostgambler]

Original geschrieben von penizillin
damit sollte man vorsichtig umgehen.
was gibt

is_numeric('5.5');
//und
is_numeric('.5'); 

zurück? in wiefern ist es erwünscht?

Ich denke mal die id wird ne ganze Zahl sein, dann kann man ja auch autoincrement nutzen usw. (hatte ich erwähnt, dass ich diese blöden Kommazahlen hasse? XD")

[sven k.]

Erstmal ein dickes Danke an marc75 und DSB.
das " war natürlich das problem!
Hatte das ganze auch in meinem PHP-Buch gelesen aber ist wohl irgendwie wieder untergegangen.


Frage an ghostgambler:

Wozu schreibst du eine Variable in eine andere Variable???

soll ich die Zeile dann also lieber so schreiben:

$titles = $xp->query('DVD[CollectionNumber='.$_GET['id'].']'); 

und warum sollte ich eine Überprüfung einbauen?
Es kann ja nur eine Ganzzahl übergeben werden, hab die seite wo die 'id' übergeben wird doch auch selbst geschrieben.
Irgendwie blicke ich das grad nicht so richtig.


Ps: Nochmals Danke an alle!!!

[ghostgambler]

Original geschrieben von sven k.
soll ich die Zeile dann also lieber so schreiben:

$titles = $xp->query('DVD[CollectionNumber='.$_GET['id'].']'); 

Yep, du siehst auf einen Blick, woher die ID kommt.

und warum sollte ich eine Überprüfung einbauen?
Es kann ja nur eine Ganzzahl übergeben werden, hab die seite wo die 'id' übergeben wird doch auch selbst geschrieben.
Irgendwie blicke ich das grad nicht so richtig.

EDIT:

---

Das folgende gilt nur für DB-Querys, du hast mich mit dem simplen Quoten von dem Query-Dings etwas verwirrt
Bei dem Query macht die Überprüfung einfach nur besseren Code, aber bei einem DB-Query, siehe unten.

---

Okay, Beispielszenario:
Ich ~ kleines dummes Blag, was nichts besseres zutun hat, als die MySQL-Datenbank anderer Leute abzuschießen, schwirre mit einer Highspeed-DSL-Flatrate im Netz rum auf der Suche nach Seiten, wo IDs übergeben werden per GET.
Ich lande bei dir, und gucke mal: Was kann ich denn da zerstören.

Ich kopieren den Link, schreiben dann hin statt:
www.deine-domain.de/index.php?get=5
z.B.
www.deine-domain.de/index.php?get='' UNION blablubber blupp
ich kenn keinen der Codes, die wirklich schadet, aber das würde z.B. dazu führen, dass dein Query so aussehen würde:
SELECT * FROM table WHERE id='' UNION und hier folgt ein anderer Query, denn mit UNION kann man Querys kombinieren.
Wenn du dann noch genügend Rechte hast, ist es dem Angreifer so möglich unter Umständen einen neuen Useraccount bei MySQL zu errichten, der Admin-Zugriff hat. D.h. der kann dann mal eben kurz alles an deiner DB ändern, oder es ist damit möglich, bei entsprechendem PHP-Code zur Ausgabe, alle Passwörter von MySQL auszugeben. usw. in der PHP-Manual existiert eine komplette Seite nur zu dem Thema, d.h. du wirst nicht drumherum kommen, wenn du sicheren Code schreiben willst, da eine Überprüfung einzubauen, jeden Text, den du einspeist via mysql_real_escape_string zu escapen, weil die Anführungszeichen damit auch escaped werden, d.h. es ist nicht möglich wie oben, den Query einfach mit einem Anführungszeichen im Link vorzeitig zu beenden. usw, so genau kenn ich mich da jetzt auch nicht aus, vielleicht können dir da andere Leute einen längeren Vortrag drüber halten, wie was und warum, ich sag dir nur einfach, dass es rein theoretisch möglich ist, und das man deshalb dagegen steuern sollte, indem man nur das durchlässt, was durch darf.
Und für $_GET['id'] ist doch ein guter Anfang zu testen, ob das Übergebene eine Zahl ist. Und auf Strings eben immer mysql_real_escape_string anwenden, siehe dazu auch
www.php.net/mysql_real_escape_string
vielleicht findest du da auch irgendwo in der Nähe die Seite über die Sicherheit bei DB-Querys *schulter zuck*

[sven k.]

Ok, jetzt hab ichs verstanden.

Hätte nicht gedacht das man doch so sehr angreifbar ist allein nur durch dieses GET.
Werde trotzdem diese Überprüfung einbauen, obwohl ich sie in meinem Fall nicht benötige.
Da ich mich noch nicht lange mit PHP beschäftige bin ich auch erstmal nur froh wenn der Code überhaupt das macht was er soll.
Sicherheitsaspekte und ordentlichen Code, so weit bin ich halt noch nicht.
Obwohl es glaub ich besser wäre es gleich richtig zu lernen.

Ei was kompliziert! (ich brauch mehr Freizeit!)


Also nochmals Danke und bis demnächst...