Query werkt wel, echter mét een waarschuwing

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

Donovan -

Donovan -

10/03/2019 15:28:37
Quote Anchor link
Hoi!

Ik wil een tabel updaten met informatie. De SQL-Query werkt prima, data word geüpdatete, toch blijft er een waarschuwing verschijnen dat er iets niet goed is in de Query.

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
$token = $_GET['token']; // vanuit de browser-url
$sql_update = $db_handle->runQuery("UPDATE gebruikers SET Geactiveerd = 'Ja' WHERE Token = '$token' ");

// daar komt uit bijvoorbeeld (gechood) SQL = UPDATE gebruikers SET Geactiveerd = 'Ja' WHERE Token = '89230c74d68fb7f2ba88'
//Foutmelding: Warning: mysqli_fetch_assoc() expects parameter 1 to be mysqli_result, boolean given in C:\xampp\htdocs\NCD\dbcontroller.php on line 25

// DB Controller functie
function runQuery($query) {
    $result = mysqli_query($this->conn,$query);
    while($row = mysqli_fetch_assoc($result))
    {
        $resultset[] = $row;
    }        
    if(!empty($resultset))
        return $resultset;
}


Dus het werkt, wel, alleen de waarschuwing blijft. Ik gebruik eerder een andere SELECT-query en die werkt prima;

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
$sql_check = $db_handle->numRows("SELECT * FROM gebruikers WHERE Token = '$token' ");

Iemand enig idee hoe dit kan? Het lijkt dus of een SELECT Query prima werkt, alleen bij een UPDATE werkt het niet..
Gewijzigd op 10/03/2019 15:43:24 door Donovan -
 
PHP hulp

PHP hulp

23/05/2019 22:05:03
 
Thomas van den Heuvel

Thomas van den Heuvel

10/03/2019 17:21:07
Quote Anchor link
PHP(.net) heeft uitstekende documentatie. Zo staat bij elke functie/methode beschreven wat deze teruggeven in verschillende gevallen (onder het kopje Return Values).

Om de documentatie maar weer te quoten:
Quote:
Returns FALSE on failure. For successful SELECT, SHOW, DESCRIBE or EXPLAIN queries mysqli_query() will return a mysqli_result object. For other successful queries mysqli_query() will return TRUE.

Trouwens het uitvoeren van een query en ophalen van resultaten zijn twee compleet verschillende dingen, waarom zou je die bij elkaar gooien in runQuery?
Gewijzigd op 10/03/2019 17:24:17 door Thomas van den Heuvel
 
Rob Doemaarwat

Rob Doemaarwat

10/03/2019 17:21:13
Quote Anchor link
Bij een UPDATE retourneert mysqli_query true indien alles OK. Daar kun je dus geen rijen uit trekken.

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
function runQuery($query) {
    $result = mysqli_query($this->conn,$query);
    if(!$result) return false; //of throw een error
    if($result === true) return true; //alles OK, maar geen resultset
    while($row = mysqli_fetch_assoc($result))
    {
        $resultset[] = $row;
    }        
    if(!empty($resultset))
        return $resultset;
}


Zoek in Google overigens ook eens naar "SQL injectie" mbt je gebruik van $token direct in de query.

Toevoeging op 10/03/2019 17:21:58:

Damn, 6 seconden te laat ;-)
 
Thomas van den Heuvel

Thomas van den Heuvel

10/03/2019 17:23:31
Quote Anchor link
En ja, om het maar niet te hebben over SQL-injectie ;).
 
Furio Scripting

Furio Scripting

13/03/2019 15:10:39
Quote Anchor link
Wat Thomas zegt, zeker even die token valideren alvorens iets met de database te doen.

Ik gebruik altijd gewoon een database class en daar kan je eenvoudig dan zo met de database communiceren:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
<?php

$database
= new database();
$q = "query";
$result = $database->query($q);

while ($row = mysqli_fetch_assoc($result)
{

echo $row['kolom naam die je wilt'];
}

[
/code]

?>
 
- Ariën -
Beheerder

- Ariën -

13/03/2019 16:00:31
Quote Anchor link
Dat is ook wat ik gebruik. Ik zelf gebruik een extended classe met MySQLi waarbij ik de foutafhandeling in de query() functie geïntegreerd heb.
 
Donovan -

Donovan -

15/03/2019 20:10:44
Quote Anchor link
Rob Doemaarwat op 10/03/2019 17:21:13:
Bij een UPDATE retourneert mysqli_query true indien alles OK. Daar kun je dus geen rijen uit trekken.

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
function runQuery($query) {
    $result = mysqli_query($this->conn,$query);
    if(!$result) return false; //of throw een error
    if($result === true) return true; //alles OK, maar geen resultset
    while($row = mysqli_fetch_assoc($result))
    {
        $resultset[] = $row;
    }        
    if(!empty($resultset))
        return $resultset;
}


Zoek in Google overigens ook eens naar "SQL injectie" mbt je gebruik van $token direct in de query.

Toevoeging op 10/03/2019 17:21:58:

Damn, 6 seconden te laat ;-)


Heb in ieder geval hier een nieuwe updateQuery functie van gemaakt.
en die injectie een nieuwe functie gemaakt. Is't wat?

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
function quote($value) {
    return mysqli_real_escape_string($this->conn, $value);
}
 
Thomas van den Heuvel

Thomas van den Heuvel

15/03/2019 20:44:08
Quote Anchor link
Donovan - op 15/03/2019 20:10:44:
en die injectie een nieuwe functie gemaakt. Is't wat?

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
function quote($value) {
    return mysqli_real_escape_string($this->conn, $value);
}

Nee, want die functie doet niet wat 'ie zegt dat 'ie doet. Deze heet "quote". De functie voegt nergens quotes toe. Een betere "shorthand" zou "escape" kunnen zijn.

Het escapen van DATA-delen in SQL is trouwens niet veilig zonder quotes, dus ook die zou je toe moeten voegen. Waar je dit doet... ik doe dit in de SQL zelf, maar als je dat in een functie of methode doet zou dit ergens in de naamgeving uit moeten blijken.
 
Furio Scripting

Furio Scripting

17/03/2019 21:55:39
Quote Anchor link
Ik raad aan om een functie te schrijven die je kan gebruiken om user input te escapen:

function check_input($data)
{

$database = new database();
$data = trim(htmlentities($data));

return mysqli_real_escape_string($database->getConnection(),$data);
}

Dit is voor mijn doeleinde meestal genoeg om te zorgen dat er geen sql injectie mogelijk is.
 
Thomas van den Heuvel

Thomas van den Heuvel

17/03/2019 22:28:31
Quote Anchor link
Ik zou bovenstaande functie niet gebruiken, en wel om de volgende redenen:
- deze past je invoer aan; mogelijk hebben spaties, regelovergangen et cetera betekenis; deze op voorhand strippen met trim() lijkt mij geen goed plan
- deze escaped invoer, dat is over het algemeen onverstandig, enkele uitzonderingen daargelaten; hierbij ga je er ook vanuit dat de data enkel (?) in een HTML-context gebruikt wordt, dit hoeft niet op voorhand vast te staan; ook wil je mogelijk rauwe HTML kunnen gebruiken en dan zou je dus de omgekeerde bewerking moeten uitvoeren; dit is allemaal niet optimaal
- deze escaped voor meerdere contexten tegelijkertijd (zowel voor HTML en voor SQL), dit is ook niet echt handig
- de naamgeving (check_input) suggereert iets anders dan wat het doet, het past mogelijk inhoudelijk $data aan; dit laatste is wat mij betreft ook onwenselijk: het is in bijna alle gevallen een beter plan om alle data "rauw" op te slaan, ongewijzigd, zoals je het aangeleverd krijgt
Quote:
Dit is voor mijn doeleinde meestal genoeg om te zorgen dat er geen sql injectie mogelijk is.

Ook dat klopt niet helemaal want het gebruik van enkel real_escape_string() is niet genoeg (interne link).

In het algemeen is het verstandiger om escaping in een bepaalde context (HTML, SQL et cetera) altijd uit te stellen tot net voor het gebruik van data in de betreffende context.
 
Furio Scripting

Furio Scripting

18/03/2019 07:47:59
Quote Anchor link
Tuurlijk Thomas, er is geen enkele functie die voor elke situatie de data kan escapen maar voor 90% van mijn velden (formuliertjes) simpele text fields is dit prima. Uiteraard check ik bij de post ook op andere zaken alvorens de data naar de database te sturen maar als basis neem ik de check_input functie en vanaf daar run ik meerdere functies erop. (strlen, strtolower, regex etc).
 
Rob Doemaarwat

Rob Doemaarwat

18/03/2019 12:42:26
Quote Anchor link
Furio Scripting op 18/03/2019 07:47:59:
... voor 90% van mijn velden (formuliertjes) simpele text fields is dit prima.


Da's fijn - voor jou (in 90% van de gevallen). Maar net als Thomas zou ik een ieder ander toch aanraden om vooral zelf na te blijven denken (in 100% van de gevallen), en niet met bovenstaande "geval" te gaan werken.
 
Thomas van den Heuvel

Thomas van den Heuvel

18/03/2019 13:35:45
Quote Anchor link
Furio Scripting op 18/03/2019 07:47:59:
er is geen enkele functie die voor elke situatie de data kan escapen

Precies, dit doe je per context en alleen als/voordat je de data gaat gebruiken. Je gaat niet op voorhand alles prepareren voor alle mogelijke contexten.

Furio Scripting op 18/03/2019 07:47:59:
maar als basis neem ik de check_input functie en vanaf daar run ik meerdere functies erop. (strlen, strtolower, regex etc).

Euh, dus je gebruikt dit ook bij het filteren van input? Maar het filteren van input en het escapen van output zijn twee compleet verschillende dingen, die zou je dus ook afzonderlijk moeten behandelen :/.

De functie slaat denk ik toch de plank mis, en zoals aangegeven is deze functie alleen nog steeds niet afdoende. Ik hoop dat je als je queries bouwt dat je naast check_input() ook quotes gebruikt in de DATA-delen van de SQL? real_escape_string() escapet namelijk niets als er niets te escapen valt. Zo zal:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
OR 1=1

ongeschonden uit real_escape_string() komen, zoals ook staat uitgelegd in de eerdere interne link in mijn vorige reactie.

Simpelweg omdat iets werkt, maakt het nog niet juist. Je moet ook weten en kunnen uitleggen waarom iets werkt.
Gewijzigd op 18/03/2019 14:15:46 door Thomas van den Heuvel
 



Overzicht Reageren

 
 

Om de gebruiksvriendelijkheid van onze website en diensten te optimaliseren maken wij gebruik van cookies. Deze cookies gebruiken wij voor functionaliteiten, analytische gegevens en marketing doeleinden. U vindt meer informatie in onze privacy statement.