Forum PHP.pl

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

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ę....

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

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 jeżeli jakikolwiek inny znak po za [0-9] zostanie wprowadzony do geta funkcja sprawdza return i cały IF zwraca błąd

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

EDIT: na czerwono

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