[PHP] MySQL injection voorkomen, wat is de manier?

Pagina: 1
Acties:

Onderwerpen


Acties:
  • 0 Henk 'm!

  • _eXistenZ_
  • Registratie: Februari 2004
  • Laatst online: 10-09 09:17
Allereerst, ik vond dat deze ook paste in Beveiliging en Virussen, dus als een modje denkt dattie daar beter past moettie maar ff gemoved...

Ik doe een paar jaar PHP scriptjes in elkaar flanzen, inmiddels heb ik een CMS waarbij ik bij een query standaard via deze methode te werk ga:

PHP:
1
2
3
4
5
6
$db->query("INSERT INTO blog_comments
            (name, postid, message, remote_addr)
            VALUES ('{$db->preparegpc($_POST['name'])}',
             '{$db->preparegpc($_POST['postid'])}',
             '{$db->preparegpc($_POST['message'])}',
             '{$_SERVER['REMOTE_ADDR']}')");


$db->preparegpc is deze functie:

PHP:
1
2
3
4
5
6
7
8
9
function preparegpc($string) {
    if (!empty($string)) { // foutje gefixt.
        if ($this->dblink == null)
            $this->connect();
        if (get_magic_quotes_gpc())
            stripslashes($string);
        return mysql_real_escape_string($string);
    }
}


Volgens mij waterdicht (Iig, nu ik vandeweek ff snel addslashes voor mysql_real_escape_string vervangen heb in mijn functie, hierom :))

Iemand die ik ken heeft het hele probleem op zijn - voor mij nog onbekende - manier getackled.
Hier bijvoorbeeld zijn inlogcode:
PHP:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
if ( $action['action'] == "adminlogtin" ) { 

    $username = $_POST['username']; 

    $password = $_POST['password'];

    if ( ereg("([0-9a-zA-Z])", $username ) AND ereg("([0-9a-zA-Z])", $password ) ) {

        $query = mysql_query("SELECT member.username, member.password FROM member WHERE username = '$username'");

        $result = mysql_fetch_row($query);  

    }

    if ( ( $result[0] == $username ) AND ( $result[1] == $password ) AND ( $username ) AND ( $password ) ) { $_SESSION['ingelogd'] = true; }

    else { $_SESSION['ingelogd'] = false; $login = 1; }

}

else if ( $action['action'] == "loguit" ) { $_SESSION['ingelogd'] = false; }


Zoals je ziet doet hij niet aan het escapen van variabelen, hij checkt gewoon met ereg of ze wel binnen een bepaalde range liggen. Zo kan je ook nooit binnenkomen, aldus hem :*)

Nu ben ik inderdaad van mening dat het vrij lastig is om nog quotes te escapen en dus een succesvolle MySQL-injection uit te voeren op deze manier, maar ik kan zo niet 1, 2, 3 bepalen of de manier ook stand zou kunnen houden tegen bijvoorbeeld de manier die ik eerder al aanhaalde, deze manier.

Hij denkt dat zijn manier net zo goed is als de mijne, ik denk dat ie best nice is maar er waarschijnlijk wel een achterdeurtje zal zitten die je niet kan overzien. Volgens mij is het beter om mijn manier te implementeren.

Wat denken jullie hiervan, en wat gebruiken jullie zelf?

Poll: Wat is de manier?
De manier van eXistenZ
De manier van orange.x
Beide manieren zijn ok.
Beide manieren zijn slecht.
Tussenstand:
Afbeeldingslocatie: http://poll.dezeserver.nl/results.cgi?pid=294662&layout=6&sort=prc
Ook een poll maken? Klik hier

N.B. Dit is dezelfde poll als die op pagina 3, heb 'm hier ook ff toegevoegd voor de volledigheid.

[ Voor 13% gewijzigd door _eXistenZ_ op 13-06-2008 10:01 . Reden: foutje in functie gefixt. ]

There is no replacement for displacement!


Acties:
  • 0 Henk 'm!

  • anonimoes
  • Registratie: Maart 2001
  • Laatst online: 11-11-2024

anonimoes

Zomerweer is ook maar relatief

Zelf gebruik ik jouw methode. (Die ik overigens uit de php manual gehaald heb) Alhoewel mijn kennis van php niet heel erg groot is denk ik dat de tweede methode toch ook waterdicht zou moeten zijn aangezien je als je alleen cijfers en letters mag gebruiken toch wel erg moeilijk een mysql of een php query kan ingeven. Ahem... Die code blijkt dus alleen maar te checken of er één letter of cijfer in staat blijkt uit een aantal van onderstaande posts, dan zou je dus wel een query kunnen geven oid...

[ Voor 24% gewijzigd door anonimoes op 03-06-2008 00:58 ]

Gemberthee: water met een smaakje.


Acties:
  • 0 Henk 'm!

  • _js_
  • Registratie: Oktober 2002
  • Laatst online: 18-08 21:31
Het if ($string) gedeelte zorgt voor een fout wanneer een waarde het getal 0 is.

Voor de rest zullen beide manier werken, jouw manier is beter, omdat het (ondanks de onmogelijkheid van 0) meer tekens toe laat. Een gebruikersnaam met een spatie, een apostrophe, een ë of een ander "speciaal" teken zal vaak genoeg voorkomen.

Acties:
  • 0 Henk 'm!

  • _eXistenZ_
  • Registratie: Februari 2004
  • Laatst online: 10-09 09:17
_js_ schreef op maandag 02 juni 2008 @ 22:37:
Het if ($string) gedeelte zorgt voor een fout wanneer een waarde het getal 0 is.

Voor de rest zullen beide manier werken, jouw manier is beter, omdat het (ondanks de onmogelijkheid van 0) meer tekens toe laat. Een gebruikersnaam met een spatie, een apostrophe, een ë of een ander "speciaal" teken zal vaak genoeg voorkomen.
Nee dan is $string 0, er zit wat in, en dus true. Als $string leeg is, dan issie false. Hij kijkt niet naar de datatype, ondanks dat de variabele $string heet ;)

There is no replacement for displacement!


Acties:
  • 0 Henk 'm!

  • BCC
  • Registratie: Juli 2000
  • Laatst online: 21:08

BCC

Ik gebruik altijd een db abstractielaag die dit voor me afhandelt. Bijvoorbeeld Adodb - activerecord doet dit prima voor je.

Ik heb zelf geen zin om alles netjes te concatten, escapen etc. etc. Je loopt dan ook altijd het risico dat je ergens 1 ding vergeet.

[ Voor 33% gewijzigd door BCC op 02-06-2008 22:44 ]

Na betaling van een licentievergoeding van €1.000 verkrijgen bedrijven het recht om deze post te gebruiken voor het trainen van artificiële intelligentiesystemen.


Acties:
  • 0 Henk 'm!

  • _js_
  • Registratie: Oktober 2002
  • Laatst online: 18-08 21:31
_eXistenZ_ schreef op maandag 02 juni 2008 @ 22:38:
[...]


Nee dan is $string 0, er zit wat in, en dus true. Als $string leeg is, dan issie false. Hij kijkt niet naar de datatype, ondanks dat de variabele $string heet ;)
C:\php>php
<?php
function preparegpc($string) {
    if ($string) {
        if ($this->dblink == null)
            $this->connect();
        if (get_magic_quotes_gpc())
            stripslashes($string);
        return mysql_real_escape_string($string);
    }
}
echo 'a'.preparegpc('0').'b';
?>
^Z
ab

Zoals je ziet, is de waarde opeens weg.

PHP doet aan loose type casting, en zet de string '0' automatisch om naar een boolean false.

Acties:
  • 0 Henk 'm!

  • crisp
  • Registratie: Februari 2000
  • Nu online

crisp

Devver

Pixelated

_eXistenZ_ schreef op maandag 02 juni 2008 @ 22:38:
[...]


Nee dan is $string 0, er zit wat in, en dus true. Als $string leeg is, dan issie false. Hij kijkt niet naar de datatype, ondanks dat de variabele $string heet ;)
PHP:
1
2
3
4
5
6
$string = '0';

if ($string)
    echo '$string is niet leeg';
else
    echo '$string is leeg';


verder in je alternatief:
PHP:
1
ereg("([0-9a-zA-Z])"

matched alles wat maar ergens een letter of cijfer bevat, dus ook dit bijvoorbeeld:
PHP:
1
$username = "' OR 'foo' = 'foo";


verder zal native POSIX regular expression support in PHP6 verdwijnen en alleen PCRE overblijven.

Intentionally left blank


Acties:
  • 0 Henk 'm!

  • Bosmonster
  • Registratie: Juni 2001
  • Laatst online: 10-09 08:45

Bosmonster

*zucht*

Jullie doen 2 verschillende dingen. Terwijl ze beiden compleet andere functies vervullen.

Je wilt ALTIJD je input escapen voor in je query. Daarnaast wil je ook nog wel eens kijken of je input wel voldoet aan wat je ervan verwacht natuurlijk. Het een sluit het ander niet uit.

Alleen optie 2 is natuurlijk complete onzin. Want wat als ik nou quotes e.d. op wil kunnen slaan :P

[ Voor 15% gewijzigd door Bosmonster op 02-06-2008 22:51 ]


Acties:
  • 0 Henk 'm!

  • _eXistenZ_
  • Registratie: Februari 2004
  • Laatst online: 10-09 09:17
We willen allebij voorkomen dat onze db injected wordt. Dat is het doel van beide functies, niet om te checken of der valid ranges ofzo zijn. Dan had ik ook echt wel ff wat anders gebruikt hoor 8)7

There is no replacement for displacement!


Acties:
  • 0 Henk 'm!

  • geez
  • Registratie: Juni 2002
  • Laatst online: 10-09 09:44
mysql_real_escape_string() is er specifiek voor gemaakt, ik zou dan ook niet weten waarom je deze niet zou gebruiken. Met de 0 zit je inderdaad wel, hoewel ik dit nog niet als probleem tegen ben gekomen. Integers imho sowieso toen met casting:

PHP:
1
$var = (int)$_GET['var'];

Acties:
  • 0 Henk 'm!

  • frickY
  • Registratie: Juli 2001
  • Laatst online: 13:55
_eXistenZ_ schreef op maandag 02 juni 2008 @ 22:26:
PHP:
1
if ( ereg("([0-9a-zA-Z])", $username ) AND ereg("([0-9a-zA-Z])", $password ) ) {
Ehm.. hij checkt alleen of de variabelen een alfanumeriek karakter bevatten, niet of ze er alleen maar uit bestaan.
Zo lek als een mandje dus

PHP:
1
if ( ereg("^[0-9a-zA-Z]+$", $username ) && ereg("^[0-9a-zA-Z]+$", $password ) ) {

of
PHP:
1
if (!ereg("[^0-9a-zA-Z]", $username ) && !ereg("[^0-9a-zA-Z]", $password ) ) {

zijn de jusite methodes, respectievelijk checken of stirng van begin tot eind uit gewenste karakters bestaat, of dat string een karakter bevat die niet gewenst is.

Sterker nog, hij declareert $result niet. Als zijn if failed valt die dus gewoon te injecteren (mits register globals aanstaat).
Nee, dit lijkt me geen goed voorbeeld, en juist een aanwijzing user-input ALTIJD te escapen, zelfs als je vind te hebben gecontroleerd of deze ongewenste karakters bevat.

[ Voor 37% gewijzigd door frickY op 02-06-2008 23:04 ]


Acties:
  • 0 Henk 'm!

  • _eXistenZ_
  • Registratie: Februari 2004
  • Laatst online: 10-09 09:17
crisp schreef op maandag 02 juni 2008 @ 22:48:

PHP:
1
2
3
4
5
6
$string = '0';

if ($string)
    echo '$string is niet leeg';
else
    echo '$string is leeg';
Ai, ga ik even mooi de mist in... :o

Nu doettie iig if (!empty(string)) \o/

There is no replacement for displacement!


Acties:
  • 0 Henk 'm!

  • orange.x
  • Registratie: Maart 2002
  • Laatst online: 18-07 10:57
@Bosmonster

De 2e optie is van mij en een voorbeeld van een voor mij alleen beveiligde pagina. Ik vind het niet nodig dat ik een quote of accolade nodig heb in mijn wachtwoord omdat het met cijfers en letters beveiligd genoeg is. Dat is 1. En je kan natuurlijk altijd nog tekens toevoegen aan je acceptatielijst. Zo ook de ë en die andere tekens met trema's etc.

Wanneer je alleen met slashes, ' OR dingen aan komt dan kom je er gewoon niet in als je geen " ' " mag gebruiken. Tenminste, dat is dus mijn visie, de ingevoerde waarde voldoet niet aan de van te voren opgestelde eisen dus helaas. Volgens frickY wel dus :)

Echter wanneer dit systeem gebruikt moet worden voor een weblog, of een forum oid, dan schiet het inderdaad misschien wat te kort omdat je mensen wel de vrijheid wil geven, of in ieder geval meer, qua wachtwoord mogelijkheden. Dus ook usernames met andere tekens.

Ah, ik lees net de reply van frickY en dat lijkt me inderdaad een betere optie :)

[ Voor 5% gewijzigd door orange.x op 02-06-2008 23:07 ]


Acties:
  • 0 Henk 'm!

  • _eXistenZ_
  • Registratie: Februari 2004
  • Laatst online: 10-09 09:17
Ik weet de site waar de code van orange.x inzit, en op een of andere manier krijg ik de zooi toch niet geinject, terwijl de code zou zeggen dat het mogelijk zou moeten zijn nu... heel apart 8)7 (vul als username in ' or 'foo' = 'foo en als password a)

There is no replacement for displacement!


Acties:
  • 0 Henk 'm!

  • EdwinG
  • Registratie: Oktober 2002
  • Laatst online: 09-09 16:54
Ik doe meestal het volgende:
PHP:
1
2
3
4
5
6
$sql = "UPDATE tabel SET
  naam = '%s',
  aantal = %d,
  score = %f
  WHERE id = %u;";
$db->query($sql, $var_met_naam, $var_met_aantal, $var_met_score, $var_met_id);

Beetje het idee van sprintf:
%d: vervangen door intval(waarde)
%s: vervangen door mysql_real_escape_string(waarde)
%f: vervangen door (float) waarde
%u: zelfde als %d, maar dan unsigned
%%: letterlijke % in de query

Reguliere expressies gebruik ik nooit voor escape of vervangen daarvan. (wel om bijvoorbeeld van een postcode de syntax te controleren)

[ Voor 13% gewijzigd door EdwinG op 02-06-2008 23:28 ]

Bezoek eens een willekeurige pagina


Acties:
  • 0 Henk 'm!

  • Bosmonster
  • Registratie: Juni 2001
  • Laatst online: 10-09 08:45

Bosmonster

*zucht*

orange.x schreef op maandag 02 juni 2008 @ 23:00:
@Bosmonster

De 2e optie is van mij en een voorbeeld van een voor mij alleen beveiligde pagina. Ik vind het niet nodig dat ik een quote of accolade nodig heb in mijn wachtwoord omdat het met cijfers en letters beveiligd genoeg is. Dat is 1. En je kan natuurlijk altijd nog tekens toevoegen aan je acceptatielijst. Zo ook de ë en die andere tekens met trema's etc.

Wanneer je alleen met slashes, ' OR dingen aan komt dan kom je er gewoon niet in als je geen " ' " mag gebruiken. Tenminste, dat is dus mijn visie, de ingevoerde waarde voldoet niet aan de van te voren opgestelde eisen dus helaas. Volgens frickY wel dus :)

Echter wanneer dit systeem gebruikt moet worden voor een weblog, of een forum oid, dan schiet het inderdaad misschien wat te kort omdat je mensen wel de vrijheid wil geven, of in ieder geval meer, qua wachtwoord mogelijkheden. Dus ook usernames met andere tekens.

Ah, ik lees net de reply van frickY en dat lijkt me inderdaad een betere optie :)
Daar gaat het toch niet om :? Als je een query samenstelt (wat je imho al maar 1x hoort te doen in je database laag) dan hoor je dat gewoon te escapen. Maakt niet uit wat voor checks je eromheen verzint. Er is geen alternatief voor escapen als het gaat om strings.

Acties:
  • 0 Henk 'm!

  • Voutloos
  • Registratie: Januari 2002
  • Niet online
geez schreef op maandag 02 juni 2008 @ 22:57:
Integers imho sowieso doen met casting:

PHP:
1
$var = (int)$_GET['var'];
Geef dan ook de reden: met mysql_real_escape_string() voor numerieke data heb je alsnog een injection mogelijkheid.
'SELECT * FROM Table WHERE ID = '.mysql_real_escape_string($input) gaat hartstikke fout als $input '1 OR 1=1' is.

Escapen gebeurt omwille een bepaalde context. mysql_real_escape_string is bedoeld voor strings in je mysql queries. Klinkt al te obvious, maar hele volkstammen denken dat deze functie de oplossing is voor alles inclusief totale wereldvrede. :/

En abstractielaag moet dus ook de context begrijpen, of die moet je aandragen zoals EdwinG laat zien dmv de verschillende printf achtige parameters.

En idd, op het punt dat je met queries bezig bent moet je de veiligheid _altijd_ kunnen garanderen. Als ergens in de code daarvoor extra checks zitten om de user aardig op foute invoer te wijzen of om onnodige acties te voorkomen is dat alleen maar mooi. :)

[ Voor 22% gewijzigd door Voutloos op 03-06-2008 08:26 ]

{signature}


Acties:
  • 0 Henk 'm!

  • user109731
  • Registratie: Maart 2004
  • Niet online
Ik doe net zoiets als EdwinG, maar dan zonder het type op te geven. De query-functie kijkt naar het type van de parameter:

• int/float word direct ingevuld.
• null word vertaald naar de string NULL
• strings gaan langs mysql_real_escape_string en worden tussen quotes gezet.

Zou hier nog iets mis kunnen gaan?

Acties:
  • 0 Henk 'm!

  • JortK
  • Registratie: Mei 2007
  • Laatst online: 26-09-2022
Zelf gebruik ik ook de methode die jij aangeeft, enkelt dan in combinatie met de sprintf(); function, ik heb er zelf nog nooit problemen mee gehad :)

Acties:
  • 0 Henk 'm!

  • user109731
  • Registratie: Maart 2004
  • Niet online
JanDM schreef op dinsdag 03 juni 2008 @ 09:13:
Zou hier nog iets mis kunnen gaan?
Om mezelf maar te quoten, het enige probleem is wanneer je bijv. een INT kolom met een string vergelijkt. Geen SQL-injectie mogelijk maar het kan wel een 'foute' query opleveren. De oplossing van EdwinG is dan misschien beter, als je dan nog automatisch de string voorziet van quotes bij %s dan is het redelijk waterdicht.

[ Voor 5% gewijzigd door user109731 op 03-06-2008 09:46 ]


Acties:
  • 0 Henk 'm!

  • Megamind
  • Registratie: Augustus 2002
  • Laatst online: 10-09 22:45
Ik doe zelf graag dit:
PHP:
1
2
3
4
5
$_REQ = array();
foreach($_GET as $key => $val)
{
     $_REQ[$key] = mysql_real_escape_string($val);
}


En dat ook voor POST en COOKIE.

Zo krijg je 1 'super global' die alle vars bevatten, en in de rest van je scripts hoef je er geen zorgen om te maken.

Acties:
  • 0 Henk 'm!

  • stijn1309
  • Registratie: December 2007
  • Nu online
Ik gebruik zelf altijd de functie mysql_real_escape_string om de waardes doorheen te halen
PHP:
1
2
3
<?php
$waarde = mysql_real_escape_string($waarde);
?>

Je moet wel verbinding hebben met je database.

[ Voor 3% gewijzigd door stijn1309 op 03-06-2008 09:56 ]


Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

@Megamind: Een beetje naief om te denken dat alle input die je krijgt alleen maar gebruikt wordt bij het inserten in de database. Dit soort 'magic_quotes' herimplementaties slaan de plank compleet mis. Data is data en die hoor je pas te transformeren naar een ander formaat op het moment dat je dat nodig hebt. mysql_real_escape doe je alleen op tekst vlak voordat je die in een query stopt en bv htmlentities doe je vlak voordat je het in he html plempt.

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

  • Megamind
  • Registratie: Augustus 2002
  • Laatst online: 10-09 22:45
Janoz schreef op dinsdag 03 juni 2008 @ 10:27:
@Megamind: Een beetje naief om te denken dat alle input die je krijgt alleen maar gebruikt wordt bij het inserten in de database. Dit soort 'magic_quotes' herimplementaties slaan de plank compleet mis. Data is data en die hoor je pas te transformeren naar een ander formaat op het moment dat je dat nodig hebt. mysql_real_escape doe je alleen op tekst vlak voordat je die in een query stopt en bv htmlentities doe je vlak voordat je het in he html plempt.
Hoezo? Het is misschien lazy ja, maar wel een stuk eenvoudiger, zeker als je het in een framework verwerkt. Zelfs al post ik een string die direct weer geprint wordt, dan heeft mysql_real_escape geen nadelige gevolgen voor de output. Ik zie geen problemen om het zo te gebruiken. Je weet 100% dat de input schoon is van alles dat uit $_REQ komt.

Acties:
  • 0 Henk 'm!

  • Voutloos
  • Registratie: Januari 2002
  • Niet online
Zie mijn opmerking over context, als je dit soort dingen doet snap je gewoon _totaal_ _niet_ waarom en waarvoor je aan het escapen bent. En magic quotes is ook officieel kut.
Megamind schreef op dinsdag 03 juni 2008 @ 10:32:
Je weet 100% dat de input schoon is van alles dat uit $_REQ komt.
Als je dus zo'n parameter als int gebruikt, zonder quotes in je query (zoals dat eigenlijk hort voor ints) heb je dus wél een injection mogelijkheid.

[ Voor 48% gewijzigd door Voutloos op 03-06-2008 10:38 ]

{signature}


Acties:
  • 0 Henk 'm!

  • Morax
  • Registratie: Mei 2002
  • Laatst online: 10-09 12:29
Voutloos schreef op dinsdag 03 juni 2008 @ 08:23:
[...]
Geef dan ook de reden: met mysql_real_escape_string() voor numerieke data heb je alsnog een injection mogelijkheid.
'SELECT * FROM Table WHERE ID = '.mysql_real_escape_string($input) gaat hartstikke fout als $input '1 OR 1=1' is.

Escapen gebeurt omwille een bepaalde context. mysql_real_escape_string is bedoeld voor strings in je mysql queries. Klinkt al te obvious, maar hele volkstammen denken dat deze functie de oplossing is voor alles inclusief totale wereldvrede. :/

En abstractielaag moet dus ook de context begrijpen, of die moet je aandragen zoals EdwinG laat zien dmv de verschillende printf achtige parameters.

En idd, op het punt dat je met queries bezig bent moet je de veiligheid _altijd_ kunnen garanderen. Als ergens in de code daarvoor extra checks zitten om de user aardig op foute invoer te wijzen of om onnodige acties te voorkomen is dat alleen maar mooi. :)
mysql_real_escape_string dekt lijkt mij wel voldoende af? Normaal gesproken zet je variabelen namelijk tussen quotes, en de mysql_real_escape_string zorgt er vervolgens voor dat die quotes niet eerder afgebroken kunnen worden, door dergelijke tekens te escapen :)

PHP:
1
2
3
4
$var = mysql_real_escape_string($_POST['name']);

$sql = "INSERT INTO USER SET Name = '" . $var . "';";
mysql_query($sql);


Dat lijkt mij redelijk afgedekt van injectie? Zo niet, dan hoor ik het graag :)

What do you mean I have no life? I am a gamer, I got millions!


Acties:
  • 0 Henk 'm!

  • Megamind
  • Registratie: Augustus 2002
  • Laatst online: 10-09 22:45
Inderdaad, het is geen good practice om variablen niet tussen ' de zetten. Dan gaat jou verhaal niet meer op.

Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

Het direct printen van een geposte string gaat alleen maar goed wanneer het toevallig net zo uit komt dat de context waarin de string gepost wordt dezelfde manier van escaping verwacht.

De input die jij uit $_REQ haalt is niet schoon. Het is in jouw geval namelijk vervuild met alllemaal slashes.

Daarnaast vernagel je ook het hele onderscheid tussen post, get en cookie variabelen.

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

  • Voutloos
  • Registratie: Januari 2002
  • Niet online
Megamind schreef op dinsdag 03 juni 2008 @ 10:39:
Inderdaad, het is geen good practice om variablen niet tussen ' de zetten. Dan gaat jou verhaal niet meer op.
Zo dan, omgekeerde wereld. Het is bad practice om integers niet als dusdanig op te schrijven. :w Maar goed, zolang je idd altijd quotes werkt zit je goed qua veiligheid, maar denk aub niet dat elke parameter een string is.

{signature}


Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

Megamind schreef op dinsdag 03 juni 2008 @ 10:39:
Inderdaad, het is geen good practice om variablen niet tussen ' de zetten. Dan gaat jou verhaal niet meer op.
Geen good practice? een quote is een teken dat het begin en het einde van een tekstveld aangeeft. Het is eerder 'lelijk' om numerieke velden te omsluiten met quotes.

hmmm... spuit 11

[ Voor 5% gewijzigd door Janoz op 03-06-2008 10:43 ]

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

  • Morax
  • Registratie: Mei 2002
  • Laatst online: 10-09 12:29
Voutloos schreef op dinsdag 03 juni 2008 @ 10:41:
[...]
Zo dan, omgekeerde wereld. Het is bad practice om integers niet als dusdanig op te schrijven. :w Maar goed, zolang je idd altijd quotes werkt zit je goed qua veiligheid, maar denk aub niet dat elke parameter een string is.
Uiteraard is niet elke parameter een string, maar ik heb mezelf wel aangeleerd om gewoon altijd quotes om mijn variabelen in queries te zetten :)
Janoz schreef op dinsdag 03 juni 2008 @ 10:42:
[...]

Geen good practice? een quote is een teken dat het begin en het einde van een tekstveld aangeeft. Het is eerder 'lelijk' om numerieke velden te omsluiten met quotes.

hmmm... spuit 11
Hmmm, zo had ik er eigenlijk nog niet naar gekeken...

[ Voor 28% gewijzigd door Morax op 03-06-2008 10:44 ]

What do you mean I have no life? I am a gamer, I got millions!


Acties:
  • 0 Henk 'm!

  • Megamind
  • Registratie: Augustus 2002
  • Laatst online: 10-09 22:45
Janoz schreef op dinsdag 03 juni 2008 @ 10:40:
Het direct printen van een geposte string gaat alleen maar goed wanneer het toevallig net zo uit komt dat de context waarin de string gepost wordt dezelfde manier van escaping verwacht.

De input die jij uit $_REQ haalt is niet schoon. Het is in jouw geval namelijk vervuild met alllemaal slashes.

Daarnaast vernagel je ook het hele onderscheid tussen post, get en cookie variabelen.
Een GET en POST worden nooit samen geset, ik kan geen voorbeelden bedenken.
Voutloos schreef op dinsdag 03 juni 2008 @ 10:41:
[...]
Zo dan, omgekeerde wereld. Het is bad practice om integers niet als dusdanig op te schrijven. :w
Ik dacht dat MySQL zelf wel kon typecasten. Ik weet zeker dat dat sneller is dan rare functies in PHP schrijven om te checken of je var wel echt een integer is.

Acties:
  • 0 Henk 'm!

  • Morax
  • Registratie: Mei 2002
  • Laatst online: 10-09 12:29
Megamind schreef op dinsdag 03 juni 2008 @ 10:48:
[...]

Ik dacht dat MySQL zelf wel kon typecasten. Ik weet zeker dat dat sneller is dan rare functies in PHP schrijven om te checken of je var wel echt een integer is.
Daar ging ik dus eigenlijk ook vanuit :)

What do you mean I have no life? I am a gamer, I got millions!


Acties:
  • 0 Henk 'm!

  • CodeCaster
  • Registratie: Juni 2003
  • Niet online

CodeCaster

Can I get uhm...

Megamind schreef op dinsdag 03 juni 2008 @ 10:48:
[...]

Een GET en POST worden nooit samen geset, ik kan geen voorbeelden bedenken.
HTML:
1
2
3
4
<form name="getpost" action="process.php?step=2" method="post">
<input type="hidden" name="step" value="3" />
<input type="submit" value="Vernaggel" />
</form>

:? ;)

[ Voor 15% gewijzigd door CodeCaster op 03-06-2008 10:53 ]

https://oneerlijkewoz.nl
Op papier is hij aan het tekenen, maar in de praktijk...


Acties:
  • 0 Henk 'm!

  • ? ?
  • Registratie: Mei 2007
  • Niet online

? ?

..

[ Voor 99% gewijzigd door ? ? op 25-01-2013 09:51 ]


Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

Megamind schreef op dinsdag 03 juni 2008 @ 10:48:
[...]

Een GET en POST worden nooit samen geset, ik kan geen voorbeelden bedenken.
Naast bovengenoemd voorbeeld waarin je ongelijk aangetoond wordt wil je in je code ten alle tijden weten of iets nu uit de GET, POST of eventueel COOKIE komt. Er is namelijk een fundamenteel verschil tussen variabelen die je via GET binnen krijgt en variabelen die je via POST binnen krijgt.
Ik dacht dat MySQL zelf wel kon typecasten. Ik weet zeker dat dat sneller is dan rare functies in PHP schrijven om te checken of je var wel echt een integer is.
Doe je niks met je data in php? Niet alle data gaat altijd rechtstreeks een database in. Waarom zou het typecasten pas op dat moment moeten gebeuren? Daarnaast kun je je beter richten op het schrijven van goed onderhoudbare en logisch opgezette code dan dit soort nutteloze micro-optimalisaties.

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

  • Voutloos
  • Registratie: Januari 2002
  • Niet online
Megamind schreef op dinsdag 03 juni 2008 @ 10:48:
Een GET en POST worden nooit samen geset, ik kan geen voorbeelden bedenken.
Zelfs al zou dat zo zijn, moet je het niet op de grote hoop knikkeren cq wel ergens controleren of bepaalde acties wel enkel via GET resp POST aangeroepen worden.
Ik dacht dat MySQL zelf wel kon typecasten. Ik weet zeker denk dat dat sneller is dan rare functies in PHP schrijven om te checken of je var wel echt een integer is.
Aannames. mysql kan idd heel aardig casten voor je, maar php checks of casts zijn zeker niet per se duurder, en zelfs al was het zo is het een kansloze micro optimalisatie ten koste van security en/of correcte datatypes.

Je geeft iig genoeg slechte argumenten om voorlopig niet meer serieus genomen te kunnen worden in security topics, dus ik ga niet verder met je kibbelen. Your loss :>

{signature}


Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

@era.zer: Wederom, waarom zou je je input al door mysql_real_escape gaan halen op het moment dat het binnenkomt? In het hele topic wordt al verteld dat het logischer is om dit juist te doen vlak voordat je het in een query zet.

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

  • ? ?
  • Registratie: Mei 2007
  • Niet online

? ?

..

[ Voor 127% gewijzigd door ? ? op 25-01-2013 09:51 ]


Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

Omdat het fout kan gaan wanneer je het wil tonen ipv in de database gooien. Het punt is dat een stuk data in je script gewoon een stuk data is, niet een voorMysqlSpeciaalVeiligGemaakteRepresentatieVanData. Als je echt wilt weten of je het niet vergeten bent dan is een aanpak als

PHP:
1
2
3
4
mysql_query(
        "INSERT INTO tabel (nummer, tekst ) values (".
        (int)$nummertje.", ".
        "'".mysql_real_escape_string($tekstje)."')");

toch veel veiliger? Dan zie je bij elke query exact of je het nu wel of niet gedaan hebt. Het speciaal voorbereiden van een variabele staat nu samen op de plek waar hij speciaal voorbereid moet zijn.

Zou je alles bovenaan doen dan bestaat de kans dat je te veel aan het escapen bent, of dat je toevallig een veld vergeten bent.

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

  • Morax
  • Registratie: Mei 2002
  • Laatst online: 10-09 12:29
era.zer schreef op dinsdag 03 juni 2008 @ 11:09:
Alles wat bij mij als userinput binnenkomt, dient om ofwel getoond te worden, ofwel in een query te gaan. Dus sowieso moet het ge-escapetet worden of htmlentities van gemaakt worden.

Dus of je het nu in het begin doet, of net voor je query? Da's toch gelijk. Ik doe het liever direct, dan vergeet ik niets en staat het duidelijk bovenaan elke pagina.
Mocht ik mijn queries anders doen (nu is het gewoon allemaal onder elkaar, zonder classes), dan zou het misschien interessanter zijn om het net voor het inserten automatisch te laten gebeuren.

Het is toch niet fout in ieder geval?
Ik mag toch aannemen dat jij andere dingen met je data doet als het getoond moet worden, dan wanneer het de database in moet ? :X

What do you mean I have no life? I am a gamer, I got millions!


Acties:
  • 0 Henk 'm!

  • ? ?
  • Registratie: Mei 2007
  • Niet online

? ?

..

[ Voor 125% gewijzigd door ? ? op 25-01-2013 09:51 ]


Acties:
  • 0 Henk 'm!

  • Bosmonster
  • Registratie: Juni 2001
  • Laatst online: 10-09 08:45

Bosmonster

*zucht*

Megamind schreef op dinsdag 03 juni 2008 @ 10:32:
[...]

Hoezo? Het is misschien lazy ja, maar wel een stuk eenvoudiger, zeker als je het in een framework verwerkt. Zelfs al post ik een string die direct weer geprint wordt, dan heeft mysql_real_escape geen nadelige gevolgen voor de output. Ik zie geen problemen om het zo te gebruiken. Je weet 100% dat de input schoon is van alles dat uit $_REQ komt.
Juist als je een fatsoenlijk framework gebruikt mag ik aannemen dat iedere laag zijn eigen taken uitvoert. De database laag zal de benodigde data escapen voor gebruik in een query. De web-weergavelaag zal de data van htmlentities voorzien (misschien heb je wel andere weergaves die dat niet benodigen).

De data bewerk je waar het voor nodig is. Niet 1x van te voren, want de ene laag weet als het goed is niet waar een andere laag de data voor gaat gebruiken.

Acties:
  • 0 Henk 'm!

Verwijderd

era.zer schreef op dinsdag 03 juni 2008 @ 11:09:
opgepast: dit lezen schaadt je gezondheid :P
Alles wat bij mij als userinput binnenkomt, dient om ofwel getoond te worden, ofwel in een query te gaan. Dus sowieso moet het ge-escapetet worden of htmlentities van gemaakt worden.

Dus of je het nu in het begin doet, of net voor je query? Da's toch gelijk. Ik doe het liever direct, dan vergeet ik niets en staat het duidelijk bovenaan elke pagina.
Ligt erg aan hoe jouw code in elkaar zit. Bij mij gaat dat allemaal rare dingen opleveren. Bijvoorbeeld als een gebruiker dmv een POST moet controleren of iets al in een database staat. Stel het staat er al in, en de gebruiker moet z'n formulier anders invullen. Dan heb a) een query nodig met POST[iets] als input en b) ik moet POST[iets] wederom invullen in het formulier (gebruikersgemak enzo).
Zou ik direct bovenaan mijn script POST[iets] door htmlentities en/of escape-functies halen, dan staat er of een ' of een \ teveel in het formulier.

Vandaar dat mijn standpunt dan ook is; aanpassen daar waar je het nodig hebt en niet eerder.
uitzondering bv: url/site/guestbook/2/
die 2 is userinput, staat voor pagina 2 (daar doe ik een berekening mee)bv: 2*20 => records limit 40,20 ofzo. In principe zou die niet ge-escapatet moeten worden. Maar het doen kan geen kwaad. Later controleer ik ook of het wel een cijfertje is.
Dat doe je later? Ik ben daar heel rigoreus in. !preg_match("/^\d+$/", $userinput) -> abort, redirect, weet ik veel, maar die pagina kan ie vergeten. Ik verwacht een cijfer, en krijg ik geen cijfer, dan kan de gebruiker direct de spreekwoordelijke boom in.
Bosmonster schreef op dinsdag 03 juni 2008 @ 11:33:
[...]

De data bewerk je waar het voor nodig is. Niet 1x van te voren, want de ene laag weet als het goed is niet waar een andere laag de data voor gaat gebruiken.
Eensch.

[ Voor 7% gewijzigd door Verwijderd op 03-06-2008 11:37 ]


Acties:
  • 0 Henk 'm!

  • Zyppora
  • Registratie: December 2005
  • Laatst online: 01-09 10:39

Zyppora

155/50 Warlock

era.zer schreef op dinsdag 03 juni 2008 @ 11:27:
je doet dat, het werkt en waarom zou je iets werkend veranderen
Dat dachten de jongens van Vtm, 2Be, Jim en Jim Mobile tot voor kort ook ;)

Overigens gebruik ik zelf een andere variant van typecasting:

PHP:
1
$var = intval($_GET['var']);    // $var verwacht een integer waarde


Lijkt me hetzelfde te doen, maar staat (imho) mooier.

Phenom II X4 945 \\ 8GB DDR3 \\ Crosshair IV Formula \\ R9 290


Acties:
  • 0 Henk 'm!

  • Morax
  • Registratie: Mei 2002
  • Laatst online: 10-09 12:29
Verwijderd schreef op dinsdag 03 juni 2008 @ 11:36:
[...]


Dat doe je later? Ik ben daar heel rigoreus in. !preg_match("/^\d+$/", $userinput) -> abort, redirect, weet ik veel, maar die pagina kan ie vergeten. Ik verwacht een cijfer, en krijg ik geen cijfer, dan kan de gebruiker direct de spreekwoordelijke boom in.
Lekkere sites bouw jij dan zeg, als de gebruiker meteen ' de boom in word gestuurd' als je ook maar iets binnenkrijgt wat je niet verwacht :X

In het geval van paginering, als ik na een intval een 0 oid binnenkrijgt, zet ik voor het gemak van de gebruiker bijvoorbeeld gewoon de pagina in de code op 1 ofzo

What do you mean I have no life? I am a gamer, I got millions!


Acties:
  • 0 Henk 'm!

Verwijderd

Morax schreef op dinsdag 03 juni 2008 @ 11:40:
[...]

Lekkere sites bouw jij dan zeg, als de gebruiker meteen ' de boom in word gestuurd' als je ook maar iets binnenkrijgt wat je niet verwacht :X

In het geval van paginering, als ik na een intval een 0 oid binnenkrijgt, zet ik voor het gemak van de gebruiker bijvoorbeeld gewoon de pagina in de code op 1 ofzo
Over het algemeen krijgt de gebruiker wel een foutmelding of iets dergelijks te zien. Als ik, bijv. in het geval van paginering (is dat een bestaand woord?) een nummer verwacht, en ik krijg een een letter of iets anders wat geen nummer is, dan ga ik er vanuit dat er een gebruiker aan het kloten is. Een normale gebruiker die netjes gebruikt maakt van de links op de website, heeft hier nooit last van.

Acties:
  • 0 Henk 'm!

  • Bosmonster
  • Registratie: Juni 2001
  • Laatst online: 10-09 08:45

Bosmonster

*zucht*

Verwijderd schreef op dinsdag 03 juni 2008 @ 11:53:
[...]


Over het algemeen krijgt de gebruiker wel een foutmelding of iets dergelijks te zien. Als ik, bijv. in het geval van paginering (is dat een bestaand woord?) een nummer verwacht, en ik krijg een een letter of iets anders wat geen nummer is, dan ga ik er vanuit dat er een gebruiker aan het kloten is. Een normale gebruiker die netjes gebruikt maakt van de links op de website, heeft hier nooit last van.
Als je de fout kunt corrigeren zonder problemen, dan heeft dat wel de voorkeur tussen het gooien van een foutmelding, alleen maar omdat jij dat leuk vindt ;) Ongeacht de reden die jij denkt dat die foutmelding veroorzaakt.

Acties:
  • 0 Henk 'm!

Verwijderd

Ik zou alle foutieve url's dan redirecten naar de homepage. Beste van 2 werelden. De fout willen corrigeren levert je nooit correcte info op omdat je niet weet wat de gebruiker heeft willen proberen. Afgezien van het feit dat incorrecte url's idd manueel verpest moeten worden door een kwaadwillende gebruiker. Daarom zeg ik, gelijk aan DarthRaider, 'PEBKAC', error.

[edit]
Gelukkig was ik het wel volledig met je eens Janoz. Ik dacht dat Bosmonster doelde op de query-string informatie (al dan niet met .htaccess gecoördineerd). Dit verstond ik onder een foutief url. Heb ik maar ff snel aangepast voor mijzelf ;)

[edit2] Vanuit een ander post, namens CodeCaster:
Little Bobby Tables
_/-\o_

[ Voor 34% gewijzigd door Verwijderd op 03-06-2008 13:51 ]


Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

Mwah, foute URL's (daarmee bedoel ik alles voor de ?) die eigenlijk geen resultaat op zouden mogen leveren horen een 404 terug te geven. In de 404 kun je vervolgens wel (clientside-) redirecten naar de homepage.

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

Verwijderd

Bosmonster schreef op dinsdag 03 juni 2008 @ 12:45:
Als je de fout kunt corrigeren zonder problemen, dan heeft dat wel de voorkeur tussen het gooien van een foutmelding, alleen maar omdat jij dat leuk vindt ;) Ongeacht de reden die jij denkt dat die foutmelding veroorzaakt.
Neuh, dat wil je eigenlijk zo veel mogelijk voorkomen net zoals de mensen voor mij al aangeven. Je dient dan bijvoorbeeld te letten op SEO issues, meerdere ingangen naar dezelfde data betekent immers een halvering of meer van je aantal hits op een bepaalde url.

Acties:
  • 0 Henk 'm!

  • Bluestorm
  • Registratie: Januari 2000
  • Laatst online: 20-08-2022
Ik wil nog even opmerken dat de
PHP:
1
2
if (get_magic_quotes_gpc())
            stripslashes($string);

in deze functie er voor zorgt dat je 'm nooit op andere data kunt gebruiken dan data die direct uit de input komen. Zelf vind ik het een stuk praktischer om te zorgen dat de stripslashes (als het niet anders kan) direct op de input wordt uitgevoerd. Dat is immers ook nodig als je user input ergens anders voor wilt gebruiken, zoals het direct weergeven in de HTML.

Verder heb je natuurlijk in een paar regels code mooie fucties die het hele query opbouwen voor je doen, zodat je ook nooit meer kunt vergeten om dingen te escapen. Zeker voor insert/update/delete is het onzin om zelf queries op te bouwen.

Tenminste... dat [ denk / zie / weet ] ik... | Javascript obfuscator | foto's en video's uploaden


Acties:
  • 0 Henk 'm!

  • Joolee
  • Registratie: Juni 2005
  • Niet online
Zyppora schreef op dinsdag 03 juni 2008 @ 11:37:
[...]


Dat dachten de jongens van Vtm, 2Be, Jim en Jim Mobile tot voor kort ook ;)

Overigens gebruik ik zelf een andere variant van typecasting:

PHP:
1
$var = intval($_GET['var']);    // $var verwacht een integer waarde


Lijkt me hetzelfde te doen, maar staat (imho) mooier.
Intval kan uit een string een cijfer halen. 'hoi05' zal 5 worden. Bij typecasting krijg je dan gewoon 0. Ik gebruik ook altijd intval.

Ik probeer altijd fouten met user input te corrigeren zover mogelijk, als iemand echt duidelijk bezig is met het proberen te hacken verwijs ik hem door naar diney.com :>

[ Voor 14% gewijzigd door Joolee op 03-06-2008 16:04 ]


Acties:
  • 0 Henk 'm!

  • Bosmonster
  • Registratie: Juni 2001
  • Laatst online: 10-09 08:45

Bosmonster

*zucht*

Bluestorm schreef op dinsdag 03 juni 2008 @ 15:40:
Ik wil nog even opmerken dat de
PHP:
1
2
if (get_magic_quotes_gpc())
            stripslashes($string);

in deze functie er voor zorgt dat je 'm nooit op andere data kunt gebruiken dan data die direct uit de input komen. Zelf vind ik het een stuk praktischer om te zorgen dat de stripslashes (als het niet anders kan) direct op de input wordt uitgevoerd. Dat is immers ook nodig als je user input ergens anders voor wilt gebruiken, zoals het direct weergeven in de HTML.

Verder heb je natuurlijk in een paar regels code mooie fucties die het hele query opbouwen voor je doen, zodat je ook nooit meer kunt vergeten om dingen te escapen. Zeker voor insert/update/delete is het onzin om zelf queries op te bouwen.
Ik heb het over de gebruikerservaring. De gebruiker moet je helpen, niet principieel tegenwerken. Ik zeg ook niet dat je nooit foutmeldingen moet geven, maar om bij IEDERE keer dat je een int verwacht maar een string (of iets anders) krijgt een foutmelding te geven en de gebruiker weer terug naar af te sturen is wat overdreven. Dan ga je er bovendien vanuit dat je applicatie foutloos werkt, want misschien kan die gebruiker er wel niks aan doen en komt het door een bug in je applicatie.

Zoals bijvoorbeeld met pagina's. Als je iets verkeerds krijgt, stuur lekker naar pagina 1.

Acties:
  • 0 Henk 'm!

  • vorlox
  • Registratie: Juni 2001
  • Laatst online: 02-02-2022

vorlox

I cna ytpe 300 wrods pre miute

Ik gebruik altijd onderstaande functie....lijkt het te doen voor mij

PHP:
1
2
3
4
5
6
7
8
9
10
public function escapeSql($data,$formInput=false)
{
    if(is_null($data))
        return 'NULL';
    if(is_numeric($data))
        return $data;
    if( $formInput && get_magic_quotes_gpc())
        $data = stripslashes($data); 
    return "'".mysql_real_escape_string($data)."'";
}


Overigens ben ik een fan van place holders..dus ik zie een query het liefst zoeits als
code:
1
SELECT id,name FROM partner WHERE id = :id


Vervolgens gebruik ik dan een een functie bindQueryValues en daar stuur ik een assoc array met waarden en de query naar toe..en de functie escaped alles. Op deze manier maak je het 1 keer en daarna is alles wat je er naartoe gooit gevalideerd..dus ook de interne queries in je code...zonder user input....hehe daar kun je ook een fout maken ;)

[ Voor 42% gewijzigd door vorlox op 03-06-2008 18:07 ]


Acties:
  • 0 Henk 'm!

  • Bluestorm
  • Registratie: Januari 2000
  • Laatst online: 20-08-2022
Bosmonster schreef op dinsdag 03 juni 2008 @ 17:35:

Ik heb het over de gebruikerservaring....

Zoals bijvoorbeeld met pagina's. Als je iets verkeerds krijgt, stuur lekker naar pagina 1.
Ik reageerde ook niet op jou, maar de oorspronkelijke code. Maar voor exact het geval dat jij beschrijft gebruik ik altijd een getRequestNumeric($key,$default=null){...} als de key niet bestaat in $_REQUEST of geen numeric is, returned ie de default. (en voor strings het equivalent wat de stripslashes er overheen gooit als php niet lekker geconfigureerd is)

Tenminste... dat [ denk / zie / weet ] ik... | Javascript obfuscator | foto's en video's uploaden


Acties:
  • 0 Henk 'm!

  • mithras
  • Registratie: Maart 2003
  • Niet online
PHP:
1
2
3
4
$partner = new Partner();
$partner->id = 1;
$partner->name = "Piet";
$partner->save();
O-)
Met desnoods nog:
PHP:
1
db::enableAutoEscape();
Als het by default uitstaat.

Oftwel: maak je in je bovenste controllerlaag nu niet druk om user input. Zoiets moet afgevangen worden in een laag die eronder zit en de directe link vormt met de database (zowel input als output). Een oplossing in de onderliggende laag zoals JanDM zegt voldoet dan prima.

Acties:
  • 0 Henk 'm!

  • Bluestorm
  • Registratie: Januari 2000
  • Laatst online: 20-08-2022
Dat voldoet precies niet met het geval wat Bosmonster aanhaalt: paging.

... LIMIT $x,$y. Daar kan net zo goed inject worden... en je wilt niet queries als ...LIMIT 'a',NULL bakken.

Overigens is de meest voorkomende foute input hierbij mensen die naar je linken en per ongeluk vergeten twee urls te scheiden door een spatie... dus voor page numbers, etc. zou matchen met een regular expression ( ^[0-9]+ )misschien nog wel vaak de oorspronkelijk bedoelde waarde opleveren, en beter resultaat geven dan standaard redirecten naar page 1 als het geen getal is.

Tenminste... dat [ denk / zie / weet ] ik... | Javascript obfuscator | foto's en video's uploaden


Acties:
  • 0 Henk 'm!

  • mithras
  • Registratie: Maart 2003
  • Niet online
Bluestorm schreef op dinsdag 03 juni 2008 @ 19:29:
Dat voldoet precies niet met het geval wat Bosmonster aanhaalt: paging.

... LIMIT $x,$y. Daar kan net zo goed inject worden... en je wilt niet queries als ...LIMIT 'a',NULL bakken.
Tja, als je toch al bezig bent met een abstractielaag:
PHP:
1
2
3
4
5
6
7
8
9
$query = "SELECT name, description FROM items";
db::enablePagination( '5' ); //5 items per page
$result = db::query( $query );
while( $pageSet = $result->getNextPage() ){
  echo "Current page: " . $result->getCurrentPage();
  foreach( $pageSet as $item ){
    echo $item->name . " - " . $item->description;
  }
}
En dan ook iets supporten als:
PHP:
1
2
3
$pageSet = $result->getPage( 2 ); //Pagina 2
$pageSet = $result->getPage( '2' ); //Cast naar int, dus pagina 2
$pageSet = $result->getPage( 'abcde' ); //Geeft 1e pagina terug
Dat is de moeite toch niet?

Acties:
  • 0 Henk 'm!

  • stunn0r
  • Registratie: Juni 2002
  • Laatst online: 03-09 09:06
Zelf gebruik ik meestal pdo http://nl2.php.net/manual/en/ref.pdo.php. Dit handelt onder andere zaken zoals het tegen gaan van sql injection af.

Acties:
  • 0 Henk 'm!

  • Patriot
  • Registratie: December 2004
  • Laatst online: 16:38

Patriot

Fulltime #whatpulsert

Bosmonster schreef op dinsdag 03 juni 2008 @ 17:35:
[...]


Ik heb het over de gebruikerservaring. De gebruiker moet je helpen, niet principieel tegenwerken. Ik zeg ook niet dat je nooit foutmeldingen moet geven, maar om bij IEDERE keer dat je een int verwacht maar een string (of iets anders) krijgt een foutmelding te geven en de gebruiker weer terug naar af te sturen is wat overdreven. Dan ga je er bovendien vanuit dat je applicatie foutloos werkt, want misschien kan die gebruiker er wel niks aan doen en komt het door een bug in je applicatie.

Zoals bijvoorbeeld met pagina's. Als je iets verkeerds krijgt, stuur lekker naar pagina 1.
Voor pagina's gaat dit op, maar voor dingen waar bepaalde gegevens a.d.h.v. een id moeten worden opgehaald (ik ga er even van uit dat dit een integer moet zijn), dan is het niet handig om iets voor te gaan schotelen waarvan je onmogelijk weet of het ook echt de bedoeling was dat de gebruiker dit kreeg.

Ook vraag ik me af in hoeverre je een gebruiker die blijkbaar zelf een url in elkaar heeft gezet tegenwerkt op het moment dat je een error laat zien om die persoon daar op te wijzen.

Acties:
  • 0 Henk 'm!

  • _eXistenZ_
  • Registratie: Februari 2004
  • Laatst online: 10-09 09:17
Het topic is een beetje verzand in een algemeen SQL injection topic, terwijl ik vooral ook wilde weten welke manier van de genoemde nu eigenlijk de way to go was. Daarom ff een polletje.
Dit ook omdat orange.x denkt dat zijn manier net zo goed is als de mijne maar ik totaal dus denk van niet :P Kannie ook ff zien wat jullie vinden :)

Poll: Wat is de manier?
De manier van eXistenZ
De manier van orange.x
Beide manieren zijn ok.
Beide manieren zijn slecht.
Tussenstand:
Afbeeldingslocatie: http://poll.dezeserver.nl/results.cgi?pid=294662&layout=6&sort=prc
Ook een poll maken? Klik hier

N.B. Dit is dezelfde poll als die in de TS, heb 'm daar voor de volledigheid ook even toegevoegd.

[ Voor 13% gewijzigd door _eXistenZ_ op 13-06-2008 10:05 ]

There is no replacement for displacement!


Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

Fuck, nu heb ik op ExcistenZ gestemd, terwijl ik alleen maar even wilde kijken wat de manier was.

Misschien gewoon in het kort zeggen wat de manieren zijn ipv van wie de manieren zijn?

Mijn eigenlijke keuze had moeten zijn: Beide methoden zijn ruk niet toereikend.. (verduidelijking 2 posts hieronder)

[ Voor 11% gewijzigd door Janoz op 13-06-2008 12:58 ]

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

  • anonimoes
  • Registratie: Maart 2001
  • Laatst online: 11-11-2024

anonimoes

Zomerweer is ook maar relatief

Janoz schreef op vrijdag 13 juni 2008 @ 10:28:
Mijn eigenlijke keuze had moeten zijn: Beide methoden zijn ruk..
Waarom zijn beide methoden slecht dan? Als ik het goed begrepen heb is de manier met mysql_real_escapestring een soort van 'best practice' wat zelfs in de online php docs staat... (daar heb ik het althans vandaan)

Gemberthee: water met een smaakje.


Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

Owh, ietsje te snel gelezen (ik had vooral de discussie in mijn hoofd, en niet de startpost). Ik dacht even dat _eXistenZ_ manier alle gpc variabelen door mysql_real_escape haalde, maar hij doet dit enkel op het moment dat ze in de query gezet worden. In principe maakt dat zijn manier alvast een stuk minder ruk :D.Het enige probleem dat er dan nog bij zijn manier blijft staan is dat ikzelf het logischer zou vinden om de stripslashes juist wel helemaal aan het begin te doen.

De imho beste manier is de input bij binnenkomst valideren zoals orange.x dat doet (en eventueel casten naar het type dat je hebben wilt waardoor een getal ook altijd een getal is en voor de rest de data te maken zoals hij is door er stripslashes overheen te halen wanneer die magic meuk aanstaat). Vervolgens met de variabelen werken en pas wanneer je ze in een query zet enkel de types escapen waarvoor dat nodig is. Eigenlijk zou je je met dit escapen niet bezig moeten houden, maar zou je gewoon een soort geparameteriseerde queries moeten gebruiken zoals ook in veel andere talen gebruikelijk is.

[ Voor 14% gewijzigd door Janoz op 13-06-2008 13:01 ]

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

Verwijderd

Waarom zet je magic_quotes niet gewoon uit...
Je hebt er niks aan werkelijk waar niet... in php6 bestaan ze al helemaal niet meer, duurt nog ff maar je kan maar voorbereid zijn...

in je php.ini als je daar bij kan/mag..
magic_quotes_gpc = 0

anders via .htaccess:
php_value magic_quotes_gpc Off

Acties:
  • 0 Henk 'm!

  • Cartman!
  • Registratie: April 2000
  • Niet online
Ik cast getallen altijd naar integers en voor strings gebruik ik deze functie:

PHP:
1
2
3
4
5
6
7
8
9
10
11
12
13
function makeDbSafeString($argStrString)
{
    // strip the slashes from the string if done with magic quotes
    if(get_magic_quotes_gpc())
    {
        $argStrString = stripslashes($argStrString);
    }
    
    // make it safe for mysql
    $argStrString = mysql_real_escape_string($argStrString);
    
    return $argStrString;
}


heeft eigenlijk altijd goed gewerkt :)

magic_quotes is idd onnodig, blij dat het gaat verdwijnen :)

Acties:
  • 0 Henk 'm!

Verwijderd

Cartman! schreef op vrijdag 13 juni 2008 @ 16:15:
Ik cast getallen altijd naar integers en voor strings gebruik ik deze functie:

PHP:
1
2
3
4
5
6
7
8
9
10
11
12
13
function makeDbSafeString($argStrString)
{
    // strip the slashes from the string if done with magic quotes
    if(get_magic_quotes_gpc())
    {
        $argStrString = stripslashes($argStrString);
    }
    
    // make it safe for mysql
    $argStrString = mysql_real_escape_string($argStrString);
    
    return $argStrString;
}
Deze functie werkt alleen goed als je er GPC-data (GET, POST, COOKIE) doorheen haalt.

Als bijvoorbeeld een RSS feed binnenhaal en deze met jouw functie wil escapen, kan het zijn dat er stripslashes wordt aangroepen terwijl dat juist niet moet; Magic Quotes GPC escaped alleen GPC-data.

Wat ik altijd doe, is bovenaan het entry point (index.php) kijken of Magic Quotes GCP is geset. Is dat zo, dan haal ik $_GET, $_POST en $_COOKIE door stripslashes heen. Zo heb je altijd de rauwe data in handen.

[ Voor 12% gewijzigd door Verwijderd op 13-06-2008 16:50 ]


Acties:
  • 0 Henk 'm!

  • Voutloos
  • Registratie: Januari 2002
  • Niet online
Verwijderd schreef op vrijdag 13 juni 2008 @ 16:49:
Als bijvoorbeeld een RSS feed binnenhaal en deze met jouw functie wil escapen, kan het zijn dat er stripslashes wordt aangroepen terwijl dat juist niet moet;
1) In die RSS feed moet uberhaupt geen addslashes escaping te zitten
2) Magic quotes moet sowieso dood
..
99) Magic quotes moet sowieso dood

{signature}


Acties:
  • 0 Henk 'm!

  • user109731
  • Registratie: Maart 2004
  • Niet online
Janoz schreef op vrijdag 13 juni 2008 @ 10:28:
Fuck, nu heb ik op ExcistenZ gestemd, terwijl ik alleen maar even wilde kijken wat de manier was.
Hier ook 8)7

Verder eens dat geparametriseerde queries beter zijn, dan hou je het centraal en kun je het nergens vergeten. Het is een kleine moeite en werkt veel sneller/veiliger.

Acties:
  • 0 Henk 'm!

  • Janoz
  • Registratie: Oktober 2000
  • Laatst online: 09-09 16:17

Janoz

Moderator Devschuur®

!litemod

Voutloos schreef op vrijdag 13 juni 2008 @ 16:58:
[...]

1) In die RSS feed moet uberhaupt geen addslashes escaping te zitten
Dat is precies zijn punt.

Met je overige 98 punten ben ik het wel eens ;)

Ken Thompson's famous line from V6 UNIX is equaly applicable to this post:
'You are not expected to understand this'


Acties:
  • 0 Henk 'm!

  • Voutloos
  • Registratie: Januari 2002
  • Niet online
Janoz schreef op vrijdag 13 juni 2008 @ 17:10:
[...]

Dat is precies zijn punt.
Dan kan stripslashes op zich geen kwaad. Je kan het zelfs proberen als feature te verkopen dat je dan geen povere dubbele escaping opslaat. :+

{signature}


Acties:
  • 0 Henk 'm!

  • Gomez12
  • Registratie: Maart 2001
  • Laatst online: 17-10-2023
Zal wel aan mij liggen, maar ik snap de discussie niet.

Ik behandel user data altijd alszijnde de data die door de user aangeleverd is.
Gooi ik de user-data in een dbase en is het een string dan mysql_real_escape_string
Gooi ik de user-data op het scherm enis het een string dan htmlentities.
Gooi ik de user-data in een txt-bestand dan de desbetreffende encoding eroverheen.

Iets als stripslashes etc standaard over de user-data heenhalen dat doe ik niet, want als de gebruiker slashes heeft opgegeven dan kan het best zijn dat het in dat uitvoerformaat gewoon gehandhaafd moet worden.

Accepteer gewoon wat er binnenkomt en ga pas filteren op het moment dat je er iets mee gaat doen en filter dan ook alleen maar op wat je gaat doen...

Zo af en toe heb ik te maken met RSS-feeds die binnengetrokken moeten worden en gelijk weer doorgezet moeten worden naar een client. Elke bewerking hierbij kan ertoe leiden dat de RSS-feed invalid wordt.
Voutloos schreef op vrijdag 13 juni 2008 @ 17:17:
[...]
Dan kan stripslashes op zich geen kwaad. Je kan het zelfs proberen als feature te verkopen dat je dan geen povere dubbele escaping opslaat. :+
Dat kan in sommige gevallen juist wel kwaad. Als jij iets rechtstreeks door wilt zetten van server 1 naar server 3 dan kan zo'n standaard stripslashes het origineel vernaggelen...

Jij noemt het een feature, ik noem het dat je geen originele output meer kunt leveren...

[ Voor 21% gewijzigd door Gomez12 op 13-06-2008 17:30 ]

Pagina: 1