Functie veilig?

Jannick · 25 November 2009

Haihai,
Ik heb hier een functie voor m'n website.. En ik vraag me af of ie veilig genoeg is. Het word gebruikt om geld van contant naar bank te overzetten. Ergens denk ik dat ik wat vergeten ben... Maar wat. Wat zou ik hier nog aan kunnen verbeteren? De meegegeven $id word al gecontroleerd voordat de methode word aangeroepen.

PHP

function cnb($hoeveel, $id)
{
    $sql = mysql_query("SELECT contant FROM users WHERE id = '" . $id ."'");
    $fetch = mysql_fetch_assoc($sql);
    if ($fetch['contant'] < $hoeveel)
    {
        return false;
    }
    else {
        $query = mysql_query("UPDATE users SET bank = bank + $hoeveel WHERE id = '" . $id ."'") or die(mysql_error());
        if ($query)
        {
        $query2 = mysql_query("UPDATE users SET contant = contant - $hoeveel WHERE id = '" . $id ."'") or die(mysql_error());
            if($query2)
            {
                return true;
            }
        }
        else { return false;
        }
    }
}

Toon Meer

Jannick

Edit: Iets in code gefixed

RiiCk · 25 November 2009

Hallo Jannick,

Na mijn weten is die wel beveiligd genoeg. Ik neem aan dat je er boven over er onder nog wel hebt staan dat ze niet meer van de bank kunnen halen dan dat ze hebben?

Groetjes

EDIT:
Ik zie dat dit er al in staat sorry, Dan lijkt die mij veilig genoeg.

Jannick · 25 November 2009

Jup, in een ander script word er een variabele aangeroepen die gekoppeld staat aan deze functie. Als die true is dan krijg je dat te zien, als ie false is een melding dat je niet genoeg geld hebt.

Niels · 25 November 2009

$id is niet beveiligd. Maak er (int) $id van of voer mysql escape real string uit.

RiiCk · 25 November 2009

Citaat van Niels

$id is niet beveiligd. Maak er (int) $id van of voer mysql escape real string uit.

Zoals jannick zegt:
De meegegeven $id word al gecontroleerd voordat de methode word aangeroepen.

Niels · 25 November 2009

Dan nog betweter. Waarom zou je extra controles plaatsen als het zo normaal toch echt moet gebeuren. met (int) weet je tenminste zeker dat het een integer is en geen decimaal getal.

Tevens zie je ook niet hoe $id 'gecontroleerd' wordt wel dus hoe kun je dan een klein scriptje bekijken op veiligheid.

Stefan.J · 25 November 2009

$hoeveel = '0, level = 255';

Denk daar maar eens over na.

Jannick · 25 November 2009

Heel leuk, maar voordat de functie word aangeroepen worden zowel de ID als valuta gecontroleerd of het numeriek is, anders roept ie 'm nieteens aan... Zou het beter zijn om het binnen de functie te controleren?

Edit:
Wat ik nu heb:

PHP

function cnb((int)$hoeveel, (int)$id)
{
    if(is_numeric($hoeveel)) {
    $sql = mysql_query("SELECT contant FROM users WHERE id = '" . $id ."'");
    $fetch = mysql_fetch_assoc($sql);
    if ($fetch['contant'] < $hoeveel)
    {
        return false;
    }
    else {
        $query = mysql_query("UPDATE users SET bank = bank + $hoeveel WHERE id = '" . $id ."'") or die(mysql_error());
        if ($query)
        {
        $query2 = mysql_query("UPDATE users SET contant = contant - $hoeveel WHERE id = '" . $id ."'") or die(mysql_error());
            if($query2)
            {
                return true;
            }
        }
        else { return false;
        }
    }
    }
}

Toon Meer

het is wel dubbel nu volgens mij, maar goed...

Edit 2: Ok, dat werkte dus niet. *zoekt verder*

Stefan.J · 25 November 2009

Die verantwoordelijkheid kun je beter binnen de functie leggen ja. Anders zit je als nog alles eerst te valideren, en dan de functie aan te roepen.

Jannick · 25 November 2009

Jup, heb 't aangepast Die (int) hoort voor de variabelen voordat ze gebruikt worden?, dus niet in bijv. function cnb($hoeveel, $id) maar in if ($fetch['contant'] < (int)$hoeveel) ?

Niels · 25 November 2009

Citaat van Jannick

Heel leuk, maar voordat de functie word aangeroepen worden zowel de ID als valuta gecontroleerd of het numeriek is, anders roept ie 'm nieteens aan... Zou het beter zijn om het binnen de functie te controleren?

Maar dan nog. Door een FAIL van php kan het altijd mis gaan. filter_var heeft bijvoorbeeld ook nog veel fouten waardoor bijvoorbeeld niet legitieme e-mail adressen toch als TRUE returnen.

Jannick · 25 November 2009

De controle voor e-mailadressen is volgens mij maximaal, er komen explodes, ereg's etc in voor, maar dat terzijde. Voor de geinteresseerden, ik heb 'm hiervan gebruikt: Klik.
Dit is het script wat ik nu heb:

PHP

function cnb($hoeveel, $id)
{
    if(is_numeric($hoeveel)) {
    $sql = mysql_query("SELECT contant FROM users WHERE id = '" . $id ."'");
    $fetch = mysql_fetch_assoc($sql);
    if ($fetch['contant'] < (int)$hoeveel)
    {
        return false;
    }
    else {
        $query = mysql_query("UPDATE users SET bank = bank + $hoeveel WHERE id = '" . $id ."'") or die(mysql_error());
        if ($query)
        {
        $query2 = mysql_query("UPDATE users SET contant = contant - $hoeveel WHERE id = '" . $id ."'") or die(mysql_error());
            if($query2)
            {
                return true;
            }
        }
        else { return false;
        }
    }
    }
}

Toon Meer

Darsstar · 25 November 2009

PHP

function cnb($boeveel, $id)
{
    $id      = (int) $id;
    $hoeveel = abs((int) $hoeveel);


    $query = mysql_query("UPDATE `users` SET `bank` = `bank` + ".$hoeveel.", `contant` = `contant` - ".$hoeveel." WHERE `contant` >= ".$hoeveel." AND `id` = ".$id);
    return (bool) mysql_affected_rows();
}

Waarom zou je PHP dingen laten controleren wanneer MySQL dit ook kan en je ook nog eens een query bespaart?

Jannick · 25 November 2009

Waar staat die abs() voor?
Nou ja, ben nog redelijk aan het leren en ben nog niet bekend met dat soort dingen
Edit: $query word nergens aangeroepen... Is dat nu ook overbodig geworden?

Darsstar · 25 November 2009

abs() geeft de absolute waarde van een getal...
-10 wordt 10
10 wordt 10

Jannick · 25 November 2009

Nice... Dit is nu wat ik heb:

PHP

function cnb($hoeveel, $id)
{
    $id      = (int) $id;
    $hoeveel = abs((int) $hoeveel);


mysql_query("UPDATE `users` SET `bank` = `bank` + ".$hoeveel.", `contant` = `contant` - ".$hoeveel." WHERE `contant` >= ".$hoeveel." AND `id` = ".$id);
    return (bool) mysql_affected_rows();
}

Typfoutje verbeterd ($boeveel). Vraag na aanleiding van je code: Zijn '-haakjes niet nodig in query's? Ik doe altijd '" . $variabele . "', en zie dat jij ze niet gebruikt. Kan je dat overal zo toepassen, en welke voorwaarden zijn er daaraan?

Darsstar · 25 November 2009

Om strings MOET je quotes zetten...
Als je mysql laat rekenen en het komt een string tegen zal het deze in een integer omzetten... (volgens mij)
Dus waarom zou je er een string van maken als mysql het daarna weer in een integer omzet...

Het "$query = " was inderdaad niet nodig...
Ik dacht eerst even dat ik de query aan mysql_affected_rows() moest geven, dat was niet zo, en toen was ik vergeten dat stukje ook nog wel te halen.

MrMees · 25 November 2009

Ik denk wel dat je nog is_numeric of ctype_digit kan gebruiken om een fout aan te geven. Dat is wel zo makkelijk voor de gebruiker!

Net zo als met hoeveelheid > contant. Als gebruiker is het wel netjes en handig om z'n fout te laten zien.

Correct me if I am wrong

Darsstar · 25 November 2009

Citaat van MrMees

Ik denk wel dat je nog is_numeric of ctype_digit kan gebruiken om een fout aan te geven. Dat is wel zo makkelijk voor de gebruiker!

Doe je mooi bij je formulier?

Citaat van MrMees

Net zo als met hoeveelheid > contant. Als gebruiker is het wel netjes en handig om z'n fout te laten zien.

Doe je als cnb() FALSE returned?

MrMees · 25 November 2009

Citaat van Darsstar

Doe je mooi bij je formulier?

Met javascript of iets dergelijks?

Partner Gezocht om meerdere NFT Collecties op Open Sea te Plaatsen

Plesk installer

cpanel / whm themes

PHP Point of Sale

Probleem omtrent Tinymce

Kunnen jullie mij helpen met deze enquete

een eigen cloud sever

Probleem C++ en CL

228 Nieuwe domeinnamen Mei 2025

Hulp bij instalatie Mafia Source

193 Nieuwe domeinnamen April 2025

Functie veilig?

Participate now!

Fijne feestdagen

Kritieke update voor Really Simple Security-plug-in

ING Nederland streeft naar ondersteuning van Google Pay tegen eind februari

Functioneel ontwerp

Access Control List implementatie in PHP/MySQL - deel 1/2

Access Control List implementatie in PHP/MySQL - deel 2/2

Delen