[SQL][PHP]Sprawdzenie poprawności skryptu Opcje

[troian]

Witam, mam napisany skrypt jednak ktoś usilnie twierdzi że można go obejść, czy ktoś mógł by zerknąć na ów skrypt i powiedzieć mi czy jest możliwość złamania/obejścia tego skryptu jeżeli ktoś widzi jakąś nieprawidłowość proszę mi powiedzieć postaram się to naprawić w miarę możliwości

[PHP] pobierz, plaintext 
function canBuy()
{
if(!isset($_SESSION['nextBuy']))
{
$_SESSION['nextBuy']=time();
}
 
if($_SESSION['nextBuy']<=time())
{
$_SESSION['nextBuy']=time()+10;
return true;
}
else
return false;
}
 
if(isset($_SESSION['id'])) {
 
if(isset($_GET['id']) && checkInt($_GET['id'])) {
 
 
$sqlCmd="SELECT id_itemu, ilosc, cena, nazwa, attrtype0, attrvalue0, attrtype1, attrvalue1, attrtype2, attrvalue2, attrtype3, attrvalue3, attrtype4, attrvalue4, attrtype5, attrvalue5, attrtype6, attrvalue6, socket0, socket1, socket2 FROM strona.itemy_itemshop WHERE id='".$_GET['id']."' AND status='1' LIMIT 1";
$sqlQry=mysql_query($sqlCmd,$polaczenie);
if(mysql_num_rows($sqlQry)==1) {
 
$getItem=mysql_fetch_object($sqlQry);
 
$sqlCoins = "SELECT coins FROM account.account WHERE id='".$_SESSION['id']."' LIMIT 1";
$qryCoins = mysql_query($sqlCoins,$polaczenie);
$getCoins = mysql_fetch_object($qryCoins);
 
 
if(canBuy())
{
if(($getCoins->coins)>=$getItem->cena) {
 
$getGroesse = compareItems($getItem->id_itemu);
$belPos = checkPos($_SESSION['user_id']);
$possiblePos = findPos($belPos['islager'],$getGroesse['groesse']);
if(!empty($possiblePos)) {
 
$nCoins = $getCoins->coins-$getItem->cena;
 
$sqlCmd="UPDATE account.account SET coins='".$nCoins."' WHERE id='".$_SESSION['id']."' LIMIT 1";
$sqlQry=mysql_query($sqlCmd,$polaczenie);
if($getItem->socket0 > 0)
{
$aktualnadata = date("Y-m-d H:i:s");
$socket0 = strtotime($aktualnadata) + $getItem->socket0;
}
$sqlItem="INSERT INTO player.item 
(owner_id,window,pos,count,vnum,attrtype0, attrvalue0, attrtype1, attrvalue1, attrtype2, attrvalue2, attrtype3, attrvalue3, attrtype4, attrvalue4, attrtype5, attrvalue5, attrtype6, attrvalue6, socket0, socket1, socket2)
VALUES 
('".addslashes($_SESSION['id'])."','MALL','".$possiblePos[0]."','".$getItem->ilosc."','".$getItem->id_itemu."','".$getItem->attrtype0."', '".$getItem->attrvalue0."', '".$getItem->attrtype1."', '".$getItem->attrvalue1."', '".$getItem->attrtype2."', '".$getItem->attrvalue2."', '".$getItem->attrtype3."', '".$getItem->attrvalue3."', '".$getItem->attrtype4."', '".$getItem->attrvalue4."', '".$getItem->attrtype5."', '".$getItem->attrvalue5."', '".$getItem->attrtype6."', '".$getItem->attrvalue6."', '".$socket0."', '".$getItem->socket1."', '".$getItem->socket2."')";
$qryItem=mysql_query($sqlItem,$polaczenie) or die(mysql_error());
		$data = date("Y-m-d H:i");
		$dodaj = mysql_query("INSERT INTO strona.zakupy SET
		data = '".$data."',
		login = '".$_SESSION['login']."',
		nazwa_itemu ='".$getItem->nazwa."',
		id_itemu = '".$getItem->id_itemu."',
		zapis='OK'
		");	
echo'
<div class="okno_informacji okno_udana">
Przedmiot pomyślnie kupiony!
</div><br>
';
}
else {
echo'
<div class="okno_informacji okno_blad">
Brak miejsca w Item Shop\'ie!
</div><br>
';
}
 
}
else {
echo'
<div class="okno_informacji okno_blad">
Masz zbyt małą ilość SM!
</div><br>
';
}
}
else
{
echo'
<div class="okno_informacji okno_blad">
Nie możesz tak często kupować!
</div><br>
';
}
}
else {
echo'
<div class="okno_informacji okno_blad">
Nie ma takiego przedmiotu w Sklepie!
</div><br>
';
}
}
else {
echo'
<div class="okno_informacji okno_blad">
Błąd skryptu!
</div><br>
';
}
echo'<p><br><br>Powrót do item shop za 5 sek, jeżeli cie nie przeniesie kliknij <a href="java script:history.back()">Powrót</a></p>
<meta http-equiv="refresh" content="4; URL=/konto/sklep">
';
}
else {
 
}
[PHP] pobierz, plaintext 

Nie proszę o napisanie gotowej poprawki proszę tylko mi wskazać w jaki sposób można tego dokonać [ Jeżeli wgl się da ] bo nie jestem pewny równie dobrze koleś może cisnąć ściemę, a ja nie jestem dokonica przekonany bo miałem jeden włam już z powodu innego drobnego błędu.

Ten post edytował troian 7.12.2014, 14:48:33

[Turson]

Słyszałeś, że można a nawet zaleca się stosowanie tabulacji w celu lepszej czytelności kodu?
Co robi funkcja checkInt()?

[troian]

Słyszałeś, że można a nawet zaleca się stosowanie tabulacji w celu lepszej czytelności kodu?
Co robi funkcja checkInt()?

Sprawdza czy wartość jest liczbowa jeżeli tak zezwala jeżeli nie blokuje całą funkcję

MAM taby tylko coś podczas kopiowania się.... (IMG:style_emoticons/default/haha.gif)

Ten post edytował troian 7.12.2014, 18:24:11

[maviozo]

Polecam przejść na PDO, zabezpieczyłeś się przed SQL Injection?
Dla mnie słabym punktem jest np. to:

WHERE id='".$_GET['id']."'

[troian]

Polecam przejść na PDO, zabezpieczyłeś się przed SQL Injection?
Dla mnie słabym punktem jest np. to:

CheckInt sprawdza czy $_GET[] jest wartością liczbową bez ', } ,) itp (IMG:style_emoticons/default/wink.gif) jeżeli jakikolwiek inny znak po za [0-9] zostanie wprowadzony do geta funkcja sprawdza return i cały IF zwraca błąd

[Sephirus]

To i tak zła praktyka z tym GETem. Staraj się raczej robić coś na zasadzie:

[PHP] pobierz, plaintext 
if(waliduj($_GET['xxx']) {
 
    $xxx = $_GET['xxx'];
 
    // ...
 
   wykonaj('SELECT * FROM costam WHERE id = '.$xxx);
 
}
[PHP] pobierz, plaintext 

Tak żeby samo $_GET nie trafiło do zapytania - kto wie czy gdzieś kiedyś tego nie skopiujesz? A jeśli będzie $xxx to wymusi przekazanie wartości i przy okazji walidację.

Ponadto - przejdź na to PDO - ułatwia walidację i pomaga w obronie przed sqlInjection (IMG:style_emoticons/default/wink.gif)

EDIT: na czerwono (IMG:style_emoticons/default/smile.gif)

Ten post edytował Sephirus 8.12.2014, 12:21:46