[PHP] krijg record niet verwijderd

[Verwijderd]

ik heb een tal van verschillende manieren geprobeerd om via php een record te verwijderen maar het lukt niet.

hieronder zie je hoe ik de id ophaal als ik op de url verwijderen druk.

PHP:

<?php 
                        
 echo '<a href="delete.php?id='.$row['itemId'].'">verwijderen</a>';
                            
?>

vervolgens heb ik deze code om de record te verwijderen, er gebeurt alleen niks en de record blijft in de database staan en er is niks gewijzigd.

PHP:

<?

require_once('db.php');
    
$cxn = @ConnectToDb($dbServer, $dbUser, $dbPass, $dbName);

$id=$_GET['itemId'];

$dsql = "DELETE FROM items WHERE itemId='$id' ";

mysql_query($dsql) or die("Mislukt om te verwijderen");

?>

eventueel kan je me db.php nog inzien, mischien is daar wel wat mee mis, maar dat geloof ik niet wat ik heb wel een een scriptje gemaakt waarmee ik records kan toevoegen.

PHP:

    /* Deze pagina bevast de connectie gegevens naar de database 
    en zorgt ervoor dat er een cookie aangemaakt wordt voor het 
    gebruik van winkelwagentje.php */

    $dbServer = "localhost";
    $dbUser = "root";
    $dbPass = "";
    $dbName = "redorangeshop";

    function ConnectToDb($server, $user, $pass, $database)
    {
        /* Connect naar de database en kijkt of het alles klopt en zo niet dan kan er niet                    geconnected worden naar de database.
        Dit komt door het gebruik van een @ voor mysql_connect en mysql_select_db. Hierdoor wordt de de fout opgevangen en
        zal er geen error op het scherm verschijnen.*/
        
        $s = @mysql_connect($server, $user, $pass);
        $d = @mysql_select_db($database, $s);
        
        if(!$s || !$d)
            return false;
        else
            return true;
    }

[ Voor 8% gewijzigd door Verwijderd op 24-03-2006 13:50 ]

[Verwijderd]

PHP:

$id=$_GET['id'];

regel 7 van je delete.php

Beetje gevaarlijk dit ook, op deze manier kan ik je hele database leeggooien

[ Voor 46% gewijzigd door Verwijderd op 24-03-2006 13:52 ]

[x-force]

en het @-teken onder druk je alle fout meldingen. Haal deze is weg, misschien krijg je wel een melding? zowiezo is het @-teken niet netjes, beter kun je wat aan foutafhandeling doen.

[Verwijderd]

Inderdaad, als je je query had afgedrukt, had je gezien dat de query niet klopt (id mist)

Programming FAQ - Algemeen

[Verwijderd]

hoe zou jij het verwijderen oplossen?

maar helaas, hij doet het nog steeds niet. geen enkele record wordt verwijderd ik begrijp er echt niks meer van wat ik fout zou kunnen doen.

[Verwijderd]

Verwijderd schreef op vrijdag 24 maart 2006 @ 14:00:
hoe zou jij het verwijderen oplossen?

maar helaas, hij doet het nog steeds niet. geen enkele record wordt verwijderd ik begrijp er echt niks meer van wat ik fout zou kunnen doen.

Druk je query even af dan... En hoe je het zou doen? Het kan wel op deze manier, maar zorg wel dat je bijv. ingelogd moet zijn als admin wil je records zo maar kunnen verwijderen op deze manier.

Het zou gewoon moeten werken, dus haal die @ weg, druk je query af en ga debuggen

[Erkens]

Verwijderd schreef op vrijdag 24 maart 2006 @ 14:00:
hoe zou jij het verwijderen oplossen?

mysql_escape_string

[NMe]

Probeer dan de @'s weg te halen, de query af te drukken, en dan hier posten als je dan zelf de fout nog niet snapt.

[Verwijderd]

Erkens schreef op vrijdag 24 maart 2006 @ 14:01:
[...]

mysql_escape_string

Kan, maar je kunt nu ook gewoon de ID in de titelbalk aanpassen

@NMe: aap me niet na

[ Voor 9% gewijzigd door Verwijderd op 24-03-2006 14:05 ]

[Erkens]

Verwijderd schreef op vrijdag 24 maart 2006 @ 14:02:
Kan, maar je kunt nu ook gewoon de ID in de titelbalk aanpassen

* Erkens keek alleen naar SQL injection

[Verwijderd]

Erkens schreef op vrijdag 24 maart 2006 @ 14:12:
[...]

* Erkens keek alleen naar SQL injection

Dat weet ik ook wel... Het is sowieso niet veilig op deze manier, dat weten we allebei wel

[Erkens]

Sowieso is het niet verstanding om GET te gebruiken bij andere acties dan gegevens ophalen. En verder moet er natuurlijk rechten gecontrolleerd worden alvorens daadwerkelijk de actie uitgevoerd wordt

Maargoed, eerst de errors weergeven

[kmf]

Even alle foute code en onveiligheden negerend:

het is jaren geleden dat ik mysql_query ipv een databaseklasse gebruik, maar een query riep je meestal aan met een connectie. Je geeft die connectie niet door. mysql_query($query, $cnx) moest je doen dus waarschijnlijk.

Maar je had het waarschijnljik zelf ook kunnen zien als je de foutmeldingen op scherm had gekwakt.

[x-force]

athlonkmf schreef op vrijdag 24 maart 2006 @ 14:33:
Je geeft die connectie niet door. mysql_query($query, $cnx) moest je doen dus waarschijnlijk.

Maar je had het waarschijnljik zelf ook kunnen zien als je de foutmeldingen op scherm had gekwakt.

Dat is niet helemaal waar, in $cnx zit alleen false / true, dus daar schiet je niets mee op.

En nog een stukje van php.net

http://nl2.php.net/manual/en/function.mysql-query.php
link_identifier

The MySQL connection. If the link identifier is not specified, the last link opened by mysql_connect() is assumed. If no such link is found, it will try to create one as if mysql_connect() was called with no arguments. If by chance no connection is found or established, an E_WARNING level warning is generated.

[ Voor 10% gewijzigd door x-force op 24-03-2006 14:40 ]

[kmf]

x-force schreef op vrijdag 24 maart 2006 @ 14:39:
[...]


Dat is niet helemaal waar, in $cnx zit alleen false / true, dus daar schiet je niets mee op.

En nog een stukje van php.net

[...]

oh, nog brakker dan ik op eerste gezicht zag dus

[Wim-Bart]

Ik zou er toch iets anders van maken:

PHP:

  // Voorkom éénvoudige SQL injection.
  // Er zijn geavanceerdere technieken, maar base64_encode/base64_decode zijn de meest 
  // elementaire.
  $sID=  base64_encode($row['itemId']);
  // Geef de id mee als GET parameter.
  echo '<a href="delete.php?id='.$sID.'">verwijderen</a>';

De delete query:

PHP:

$dbServer = "localhost"; 
$dbUser = "root"; 
$dbPass = ""; 
$dbName = "redorangeshop"; 

// Open de database en retourneer de "connectie" id. 
// Is nodig omdat je door fout programmeren of juist bewust soms meerdere 
// actieve connecties kunt hebben. Gebruik in mysql_commando's dus altijd die ID!!!!!
$connectie = ConnectToDb($dbServer, $dbUser, $dbPass, $dbName); 

// Haal de parameters vanaf de server.
// Kijk of er een GET is geweest met de parameter 'id', zo ja, stop die dan in een variabele
// zo niet doe dan het zelfde voor een POST.
// ook geen POST dan FALSE zetten.
$sID= (isset($_GET['id'])?$_GET['id']:(isset($_POST['id'])?$_POST['id']:FALSE));

// Bij FALSE was er dus geen GET of POST. Let op de === (dus geen ==)
if ($sID===FALSE) 
{
  die('Parameter id: heeft geen waarde.');
}
else
{
  // Decodeer nu de id met base64_decode (we hadden hem verzonden met base64_encode).
  $iID= base64_decode($sID);
  // Stel de query samen. Gebruik ` (backtics) om de tabel en veldnamen heen. Is veiliger
  // en voorkomt conflicten met MySql keywords.
  // Haal ook de ID door real_escape_string, voorkomt basis sql injection attacks. 
  // Overigens mogen mysql getallen ook tussen ' (quote) staan. Leer je tevens aan om een ; te
  // gebruiken achter je statement. Deze laatste twee voorkomen ook basis SQL injection attacks.
  $sSql = "DELETE FROM `items` WHERE `itemId`='".mysql_real_escape_string($iID)."';"; 
  // Voer nu de code uit. Gebruik geen @, heeft totaal geen nut.
  // Let op! Gebruik van de $connectie.
  if (mysql_query($sSql,$connectie))
  {
    echo "Gelukt!";
  }
  else
  {
    // De fout als het mislukt is.
    die("Mislukt! SQL Error: ".mysql_error($connectie));  
  }
}

function ConnectToDb($server, $user, $pass, $database) 
{ 
  // Open een connectie, hergebruik eerdere connecties, voorkomt dat je meerdere
  // verbindingen krijgt vanuit 1 sessie naar de zelfde server/user/pass.        
  if ($connectie=@mysql_connect($server, $user, $pass, TRUE))
  { 
    // Gelukt, dan heeft $connectie een andere waarde dan FALSE
    if (mysql_select_db($database, $connectie))
    {
      return $connectie;
    }
    else
    {
      die('SQL Error: '.mysql_error($connectie));         
    }
  } 
  else
  {
    die('Connect fout opgetreden: '.mysql_error());
  }
}

[ Voor 52% gewijzigd door Wim-Bart op 24-03-2006 15:10 ]

[Erkens]

wim-bart schreef op vrijdag 24 maart 2006 @ 14:49:
Ik zou er toch iets anders van maken:

PHP:

  $sID=  base64_encode($row['itemId']);
  echo '<a href="delete.php?id='.$sID.'">verwijderen</a>';

base64? waarom? beetje onzinnig voor een simpele ID
dat bied geen enkele meerwaarde hier aangezien jouw code toch geen rekening houdt met security

verder zou ik de mysql statements niet zo doen maar met 'or die(mysql_error())' oid ipv in een if statement aangezien je zo alsnog de E_WARNING krijgt

[x-force]

wim-bart schreef op vrijdag 24 maart 2006 @ 14:49:
php code

misschien offtopic maar mag ik vragen waarom je gebruikt maakt van base64_encode() ? Wat is daarvan het voordeel? Als aanvaller kan ik dit nogsteeds gemakkelijk vervangen door mijn eigen ge-encode string.

_{ik lees graag andermans code om weer nieuwe dingen te leren}

[Janoz]

Dat base_64 encoden heeft geen enkel nut. Het is een simpele vorm van het moeilijker laten lijken die eigenlijk nergens op slaat.

base64 encoden is alleen maar bedoeld om een binair stuk data te kunnen versturen via een tesktueel medium (zoals mail).

Daarnaast is het controleren maar een klein deel van het security probleem. Als ik een admin over kan halen om naar een bepaalde link te surfen heb je dat ook alweer omzeilt. Zeker als je get parameters gebruikt is het ubersimpel om bijvoorbeeld gewoon een plaatje of een avatar te nemen met de betreffende link. Zodra de admin dan op die pagina komt wordt er een request uitgevoerd en ben je al klaar.

Sowieso is de oplossing van het probleem al in de eerste post gegeven. Het lijkt me namelijk redelijk lastig om een itemId te lezen uit een query string die bestaat uit http://bla.bla/bla.php?id=1.

[GX]

x-force schreef op vrijdag 24 maart 2006 @ 14:39:
[...]


Dat is niet helemaal waar, in $cnx zit alleen false / true, dus daar schiet je niets mee op.

En nog een stukje van php.net

[...]

Beter leren lezen:

mysql_connect
Return Values

Returns a MySQL link identifier on success, or FALSE on failure.

Dat jij een link identifier interpreteert als true is jouw zorg; maar je kan dus wel meerdere verbindingen tegelijk open hebben en dan de queries naar de juiste lijn sturen met de link-identifier.

[Wim-Bart]

x-force schreef op vrijdag 24 maart 2006 @ 14:54:
[...]


misschien offtopic maar mag ik vragen waarom je gebruikt maakt van base64_encode() ? Wat is daarvan het voordeel? Als aanvaller kan ik dit nogsteeds gemakkelijk vervangen door mijn eigen ge-encode string.

_{ik lees graag andermans code om weer nieuwe dingen te leren}

Als je het commentaar leest dan zie je dat er ook staat dat het een elementaire beveiliging is meer gericht tegen gewone gebruikers. Zie het als kleine kinderen, ze zien staan in de url "/wissen.php?id=2" of "weergeven.php?id=8", wat doen ze, ze veranderen het nummertje en je hebt shit. Geef je echter een base64_encode dan wordt het al wat moeilijker want het is opeens onleesbaar. Het zelfde geldt voor het gebruik van mysql_real_escape_string en quotes.

Stel: http://mijnsite/winkelwagentje.php?ordernummer=8129&actie=wis
Wat als je nu geeft http://mijnsite/winkelwagentje.php?ordernummer=8129%20or%20(false=false)&actie=wis

Met base64_encode of een eigen encryptie functie in combinatie (laatste heeft voorkeur) wordt het nog beter. Wat is de url http://mijnsite/winkelwagentje.php?ordernummer=HSseuJEW&actie=HdHKHEsha
Bij dit laatste moet je doelbewust gaan zoeken. Zeker als er nog één vreemd algorithme achter hangt.

Wanneer je je site al opzet met enkel base64_encode en base64_decode dan is het later makkelijk aan te passen. Je schrijft twee eigen functies bijvoorbeeld mysecure_encrypt en mysecure_decrypt en je doet een zoek en vervang over je hele site en klaar ben je. Een goed design houdt namelijk rekening met toekomstige eisen en wensen indien deze mogelijk zijn.

Erkens schreef op vrijdag 24 maart 2006 @ 14:52:
[...]
verder zou ik de mysql statements niet zo doen maar met 'or die(mysql_error())' oid ipv in een if statement aangezien je zo alsnog de E_WARNING krijgt

Juist daarom moet je dus dat soort constructies gebruiken. Ook bijna alle warnings zijn fouten! De @ en andere constructies zijn leuk, maar fout afhandeling doe je in je code en laat je niet onderdrukken door truukjes. De warnings welke je uiteindelijk nog krijgt en geaccepteerd zijn als onvermijdelijk suppress je door E_WARNING UIT te zetten in je PHP.INI op je productie server. Op een productieserver welke aan het internet hangt wil je totaal geen fouten of meldingen en heb je in principe alleen maar E_COMPILE_ERROR|E_ERROR|E_CORE_ERROR|E_PARSE aanstaan, andere zaken kunnen alleen maar problemen bloot leggen aan mensen waarvan je niet wilt dat ze deze meldingen zien. Zijn zelfde catagorie "gebruikers" welke ook zelf base64_waarden kunnen lezen en genereren.

[ Voor 25% gewijzigd door Wim-Bart op 24-03-2006 15:33 ]

[Erkens]

wim-bart schreef op vrijdag 24 maart 2006 @ 15:24:
Als je het commentaar leest dan zie je dat er ook staat dat het een elementaire beveiliging is meer gericht tegen gewone gebruikers. Zie het als kleine kinderen, ze zien staan in de url "/wissen.php?id=2" of "weergeven.php?id=8", wat doen ze, ze veranderen het nummertje en je hebt shit.

Daarom ook geen GET hiervoor gebruiken in de eerste plaats...

Geef je echter een base64_encode dan wordt het al wat moeilijker want het is opeens onleesbaar. Het zelfde geldt voor het gebruik van mysql_real_escape_string en quotes.

Stel: http://mijnsite/winkelwagentje.php?ordernummer=8129&actie=wis
Wat als je nu geeft http://mijnsite/winkelwagentje.php?ordernummer=8129%20or%20(false=false)&actie=wis

Met base64_encode of een eigen encryptie functie in combinatie (laatste heeft voorkeur) wordt het nog beter. Wat is de url http://mijnsite/winkelwagentje.php?ordernummer=HSseuJEW&actie=HdHKHEsha
Bij dit laatste moet je doelbewust gaan zoeken. Zeker als er nog één vreemd algorithme achter hangt.

voor dit soort dingen is juist mysql_real_escape_string bedoeld, daarmee is het praktisch onmogelijk om dit soort rare dingen uit te voeren (numerieke waarden moet je overigens wel van te voren casten naar een nummer, aangezien deze niet tussen quotes gaan)
Met base64 los je niks op.

Wanneer je je site al opzet met enkel base64_encode en base64_decode dan is het later makkelijk aan te passen. Je schrijft twee eigen functies bijvoorbeeld mysecure_encrypt en mysecure_decrypt en je doet een zoek en vervang over je hele site en klaar ben je. Een goed design houdt namelijk rekening met toekomstige eisen en wensen indien deze mogelijk zijn.

een goed design zorgt ervoor dat je geen onzinnige dingen als base64 nodig hebt in eerste plaats.

[Wim-Bart]

Erkens schreef op vrijdag 24 maart 2006 @ 15:29:
voor dit soort dingen is juist mysql_real_escape_string bedoeld, daarmee is het praktisch onmogelijk om dit soort rare dingen uit te voeren (numerieke waarden moet je overigens wel van te voren casten naar een nummer, aangezien deze niet tussen quotes gaan)

Gelukkig kun je bij MySql gewoon getallen tussen quotes zetten en kan MySql er goed mee omgaan. Een stukje uit de MySql manual:

Remember to check numeric data as well. If an application generates a query such as SELECT * FROM table WHERE ID=234 when a user enters the value 234, the user can enter the value 234 OR 1=1 to cause the application to generate the query SELECT * FROM table WHERE ID=234 OR 1=1. As a result, the server retrieves every record in the table. This exposes every record and causes excessive server load. The simplest way to protect from this type of attack is to use apostrophes around the numeric constants: SELECT * FROM table WHERE ID='234'.

bron: MySql manual: 5.3.1 General Security Guidelines


Misschien moeten mensen maar eens Webprogrammers hacking guide van Sijmen Ruwhof lezen.

[ Voor 80% gewijzigd door Wim-Bart op 24-03-2006 15:46 ]

[Erkens]

wim-bart schreef op vrijdag 24 maart 2006 @ 15:42:
Misschien moeten mensen maar eens Webprogrammers hacking guide van Sijmen Ruwhof lezen.

Ik heb het even doorgelezen, maar ik zie vooral veel onjuistheden en foute/domme tips die daar gegeven worden. Tuurlijk een deel is prima te gebruiken, maar ga het alsjeblieft niet als handleiding, en al helemaal niet als "waarheid"

[GX]

wim-bart schreef op vrijdag 24 maart 2006 @ 15:42:
Gelukkig kun je bij MySql gewoon getallen tussen quotes zetten en kan MySql er goed mee omgaan. Een stukje uit de MySql manual:

Dat iets kan betekend gelukkig niet dat het zo hoort.

Wat aanvullender:
Als je graag controle wilt op wat de user invoert moet je je formulier-afhandeling beter regelen; en niet met dat soort vieze truukjes aan de slag gaan. Dat maakt het geheel ook wat fijner om te onderhouden, en soms handiger als je bijvoorbeeld overwilt naar een andere database.

[ Voor 34% gewijzigd door GX op 24-03-2006 16:13 ]

[EdwinV]

Verwijderd schreef op vrijdag 24 maart 2006 @ 13:49:

PHP:

<?php           
echo '<a href="delete.php?id='.$row['itemId'].'">verwijderen</a>';                  
?>

PHP:

<?
// ..
$id=$_GET['itemId'];
// ..
?>

..

De goede tips over beveiliging en mysql_error zijn allemaal volledig terecht. De reden waarom in dit script de query niet zal slagen, is dat in de link wordt verwezen naar delete.php?id=xx, terwijl in delete.php itemId verwacht wordt.

Zet voortaan de error reporting van je script wat hoger zodat je hier netjes een notice van krijgt.

[Verwijderd]

Flydesign.nl schreef op vrijdag 24 maart 2006 @ 16:14:
[...]


De goede tips over beveiliging en mysql_error zijn allemaal volledig terecht. De reden waarom in dit script de query niet zal slagen, is dat in de link wordt verwezen naar delete.php?id=xx, terwijl in delete.php itemId verwacht wordt.

Zet voortaan de error reporting van je script wat hoger zodat je hier netjes een notice van krijgt.

En... dat had ik in post 2 al gezegd

[Wim-Bart]

GX schreef op vrijdag 24 maart 2006 @ 16:05:
[...]


Dat iets kan betekend gelukkig niet dat het zo hoort.

Wat aanvullender:
Als je graag controle wilt op wat de user invoert moet je je formulier-afhandeling beter regelen; en niet met dat soort vieze truukjes aan de slag gaan. Dat maakt het geheel ook wat fijner om te onderhouden, en soms handiger als je bijvoorbeeld overwilt naar een andere database.

Dat ben ik helemaal met je eens. Ik vindt uberhaubt al dat je GET niet niet moet gebruiken maar voornamelijk met post moet werken. Daarnaast moet je zo veel mogelijk standaarden aanhouden zodat je makkelijk kan switchen naar een andere database. Daar is SQL ook voor bedoeld. Portable code heeft natuurlijk user checking in de form zelf zitten. Maar ja, voor de meeste mensen is te veel werk. En ik moet toegeven, als ik zelf snel een form in elkaar zet, is input checking het laatste wat ik toevoeg. Dat komt later wel, want de specs van een form kunnen ook wijzigen.

Er zijn overigens vele methoden om een form te checken en ook de oorsprong van een form. Zelf gebruik ik nooit het sessie object maar houd ik bijvoorbeeld een database bij waarin ik een uniek id bij houd van client IP, sessie ID en een unieke hash welke ik als hidden parameter meegeef en na de post check ik daarop. 2 keer een post met zelfde hash, ip en sessieid wordt zo ondervangen. Daarnaast gebruik ik (misschien te veel) het settype() van php om de form parameters te filteren voor ik ze een database inpomp.

Kortom, er zijn een groot aantal mogelijkheden en iedereen kent zo zijn middelen en truukjes. Zelf houd ik mezelf voor dat het meest veilige bestaat uit een aantal basiscomponenten waaronder: Gegevens moeten zoveel mogelijk niet interpreteerbaar zijn voor de gemiddelde gebruiker van de webapplicatie, alles wat mogelijk versleuteld kan worden tussen client en server moet ook worden versleuteld en er mag geen directe relatie zijn tussen userdata uit de interface en onderliggende database, er moet een laag tussenzitten die de boel valideerd en uiteindelijk een valide SQL commando opleverd welke zowel voor MySql, Oracle, MS Sql en andere SQL's werkt.

[x-force]

GX schreef op vrijdag 24 maart 2006 @ 15:17:
[...]

Beter leren lezen:

[...]


Dat jij een link identifier interpreteert als true is jouw zorg; maar je kan dus wel meerdere verbindingen tegelijk open hebben en dan de queries naar de juiste lijn sturen met de link-identifier.

Ik bedoel zoals hij het gebruikt, hij geeft een true/false terug van de functie en niet de link identifier.

Verwijderd schreef op vrijdag 24 maart 2006 @ 13:49:

PHP:

    function ConnectToDb($server, $user, $pass, $database)
    {
    
        $s = @mysql_connect($server, $user, $pass);
        $d = @mysql_select_db($database, $s);
        
        if(!$s || !$d)
            return false;
        else
            return true;
    }

_{En bedankt voor de uitleg van base64}

[ Voor 10% gewijzigd door x-force op 24-03-2006 16:29 ]

[EdwinV]

Verwijderd schreef op vrijdag 24 maart 2006 @ 16:18:
[...]

En... dat had ik in post 2 al gezegd

Ik kon je verwijzing naar die regel al niet helemaal plaatsen, ik dacht dat je doelde op de veiligheid. Excuses.