Goedenavond,

Ik zit met een klein probleempje, ik heb een eigen online mafia spel, met een kennis.

Helaas hebben wij last van iemand die continu sql injections op 1 a 2 pagina's zit uit te proberen, en met success.

Deze code maakt het mogelijk om gebruikers wapens in het spel te kopen, zodat ze die kunnen gebruiken voor battles etcetera.

Dat gaat via deze code
-----------------------------------
<?
include '../header.php';

if (isset($_GET['buy'])) {
$_GET['buy'] = (int)$_GET['buy'];

$resultnew = mysql_query("SELECT * from `items` WHERE `id` = '".$_GET['buy']."' and `buyable` = '1'");
$worked = mysql_fetch_array($resultnew);
$invent = Check_Invent($user_class->id);
if($worked['id'] != ""){
if ($user_class->money >= $worked['cost']){
if ($invent < $user_class->invent) {
$newmoney = $user_class->money - $worked['cost'];
$newsql = mysql_query("UPDATE `grpgusers` SET `money` = '".$newmoney."' WHERE `id`= '".$user_class->id."'");
Give_Item($_GET['buy'], $user_class->id);//give the user their item they bought
echo Message("You have purchased a ".$worked['itemname'].".");
} else {
echo Message("Your inventory is full.");
}
} else {
echo Message("You do not have enough money to buy a ".$worked['itemname'].".");
}
} else {
echo Message("That isn't a real item.");
}
}

include('../buyitem.php');

$result = mysql_query("SELECT * FROM `items`");
$howmanyitems = 0;
while($line = mysql_fetch_array($result, MYSQL_ASSOC)) {

if ($line['offense'] > 0 && $line['buyable'] == 1){
$weapons .= "

<td width='25%' align='center'>

<img src='/static/". $line['image']."' width='100' height='100' style='border: 1px solid #000000'><br>
". item_popup($line['itemname'], $line['id']) ."<br>
$". prettynum($line['cost']) ."<br>
<form method='POST'>
<input type='text' name='quantity' size='2' value='1' onKeyPress='return numbersonly(this, event)' />
<input type='hidden' name='buy' value='{$line['id']}' />
<input type='submit' value='Buy' />
</form>
</td>
";
$howmanyitems = $howmanyitems + 1;
if ($howmanyitems == 4){
$weapons.= "</tr><tr height='15'></tr><tr>";
$howmanyitems = 0;
}
}
}
if ($weapons != ""){
?>
<tr><td class="contentspacer"></td></tr><tr><td class="contenthead">Weapons Store</td></tr>
<tr><td class="contentcontent">
<table width='100%'>
<tr>
<? echo $weapons; ?>
</tr>
</table>
</td></tr>
<?
}
include '../footer.php'
?>
-----------------------------------

De buyitem.php code bevat:

<?php
if (isset($_POST['buy']))
{
$item_id = (int)$_POST['buy'];
$item_quantity = (int)$_POST['quantity'];

$item_rs = mysql_query("SELECT * FROM `items` WHERE `id`={$item_id} AND `buyable`=1");
if ($item_rs == FALSE)
{
echo Message(mysql_error());
}
else if ($item_quantity <= 0)
{
echo Message("Invalid quantity.");
}
else if (mysql_num_rows($item_rs) == 0)
{
echo Message("That isn't a real item.");
}
else
{
$item_row = mysql_fetch_assoc($item_rs);
$occupied_invent = Check_Invent($user_class->id);
$total_cost = $item_row['cost'] * $item_quantity;

if ($user_class->money < $total_cost)
{
echo Message("You do not have enough money to buy a {$item_row['itemname']}.");
}
else if ($user_class->invent <= $occupied_invent)
{
echo Message("Your inventory is full.");
}
else if ($user_class->invent < ($occupied_invent + $item_quantity))
{
echo Message("You don't have space in your inventory for that.");
}
else
{
$user_class->money -= $total_cost;
$money_rs = mysql_query("UPDATE `grpgusers` SET `money`=`money`-{$total_cost} WHERE `id`={$user_class->id}");
if ($money_rs)
{
Give_Item($item_id, $user_class->id, $item_quantity);
if ($item_quantity == 1)
echo Message("You have purchased a {$item_row['itemname']}.");
else
echo Message("You have purchased {$item_quantity} x {$item_row['itemname']}.");
}
}
}
}
?>

Ik heb verschillende mogelijkheden geprobeerd, zoals:

<?php
if (!is_numeric($_POST['buy'])) {
echo Message("You can only insert numbers into this field.");
}
?>

en de $_POST tags heb ik geprobeerd om met mysql_real_escape_string te beveiligen, helaas heeft dit ook niet geholpen, zoals dit:

<?php
$_POST['buy'] = mysql_real_escape_string($_POST['buy']);
?>

Na 2 dagen hier aan gewerkt te hebben ben ik einde raad.. Hoe kom ik er op dat deze pagina fouten bevat?
We hebben zijn ip nagekeken en door de acceslogs heen gehaald, en de enigste pagina's die de persoon opent zijn deze 2.

Weet iemand het probleem hier van? Ik ben echt ten einde raad.

Met vriendelijke groet,
Robert-Jan
Als je goed systematishc in de hele code van je site nou elke $_POST en $_GET en $_COOKIE variabele door mysql_real_escape_string() haalt, kan er geen injectie meer mogelijk zijn. Of beter zelfs, verdiep je eens in prepared statements......
Ik ben niet z n kenner met sqlinjectie maar ik zou bij regel 5 van je bovenste code if(ctype_digit($_get['buy'])) gebruiken om te zien of de get wel een getal is

Je probleem kan overigens ook heel ergens anders zitten. Als iemand de tabelstructuur van je database weet kan hij het proberen in elk invoer veld. Bijv in een privebericht of chat, zelfs registreer velden en noem maar op. Dus zoals -Aar- al zegt. Je hele site opnieuw nalopen

Hoe kom je erbij dat t vanaf die een of twee paginas vandaan komt
dit is geen SQL injection eigenlijk. Wat er gebeurt is dat je negatieve getallen toestaat. En zoals je weet is - en - een plus. Koop maar eens -10 wapens, als het goed is krijg je dan geld bijgeschreven op je account.

is_numeric moet je vervangen door ctype_digit()
Dus dit zou moeten werken? Bedankt voor de hulp :)

<?php
foreach($_POST as $part) {
mysql_real_escape_string($part);
}
?>

Hoe kom je erbij dat t vanaf die een of twee paginas vandaan komt

Omdat we zijn IP hebben nagekeken in de acceslogs om te zien op welke pagina's dit gebeurt.
Dat werkt alleen als $part een string is maar het kan ook een array zijn.
- Mark - op 30/01/2013 19:54:33

dit is geen SQL injection eigenlijk. Wat er gebeurt is dat je negatieve getallen toestaat. En zoals je weet is - en - een plus. Koop maar eens -10 wapens, als het goed is krijg je dan geld bijgeschreven op je account.

is_numeric moet je vervangen door ctype_digit()


Ik heb zojuist -10 geprobeerd, maar dan krijg ik invalid quantity. Wat ook de bedoeling is daarvan. :)
:p sorry, ik had over regel 12 gekeken.
- Mark - op 30/01/2013 19:54:33

dit is geen SQL injection eigenlijk. Wat er gebeurt is dat je negatieve getallen toestaat. En zoals je weet is - en - een plus. Koop maar eens -10 wapens, als het goed is krijg je dan geld bijgeschreven op je account.

is_numeric moet je vervangen door ctype_digit()


T is ook leuk om abs() te gebruiken, voor de slimmerikken die bijdehand willen doen, dan hebben ze toch wel 10 wapens gekocht :)
RobertJan Deckers op 30/01/2013 19:57:30

Dus dit zou moeten werken?

<?php
foreach($_POST as $part) {
mysql_real_escape_string($part);
}
?>


Nee.
Hier maak je steeds een nieuwe $part aan die je probeert te beveiligen.
Echter met $_POST gebeurt helemaal niks.
Oftewel zinloos stukje script.
Zou je me de link naar je spel kunnen sturen per pm? Ik wil wel eens kijken hoe het eruit ziet en een paar dingetjes proberen die ik me nog vaag herinner van vroeger.

Reageren