Page 1 of 1

Please take a look at my code

Posted: Tue Oct 28, 2008 7:58 pm
by cdoyle
Hi,
I've been working on something new for my game, and been coding this from scratch.

It all seems to work, but I'm worried about security and the more I read the more confused I get on what I need to do to make sure nobody does anything to my db.

Here is what I have so far, what else do I need to do to the GET and POSTS?

Code: Select all

 
<?php
 
    include("lib.php");
    define("PAGENAME", "CAC Public Market");
    $player = check_user($secret_key, $db);
    $getcategories =$db->execute("SELECT `Weapon_Type`, `ID` FROM weapon_type");
    $getmarket=stripslashes($_GET['catid']);
    $viewmarket = $db->execute("SELECT c.market_ID as mID, c.Item_Sold_ID, c.Seller_ID, c.Price, c.days_left, p.username, p.id as pid, b.id as wid, b.name as wname, b.type as wtype FROM market_new c 
INNER JOIN players p ON c.Seller_ID = p.id
INNER JOIN blueprint_items b ON c.Item_Sold_ID = b.id 
Where b.type=?", array($getmarket));
    include("templates/private_header.php");
    
    
    switch ($_GET['act']) 
    {
       
        case "cat":
        if ($viewmarket->recordcount() == 0)
        { 
            echo "Sorry, No items for sale in this Market <!-- s:( --><img src=\"{SMILIES_PATH}/icon_sad.gif\" alt=\":(\" title=\"Sad\" /><!-- s:( -->";
        }
        else 
        {
            echo "<table width=\"100%\" border=\"1\">";
            echo "<tr align=\"center\">";
            echo "<td><h3>Item</h3></td>";
            echo "<td><h3>Seller</h3></td>";
            echo "<td><h3>Cost</h3></td>";
            echo "<td><h3>Days Left</h3></td>";
            echo "<td><h3>Purchase</h3></td>";
            while ($marketview = $viewmarket->fetchrow())
            {
                echo "<tr>";
                echo "<td>" . $marketview['wname'] . "</td>";
                echo "<td>" . $marketview['username'] . "</td>";
                echo "<td>" . $marketview['Price'] . "</td>";
                echo "<td></td>";
                echo "<td><a href=\"marketnew.php?act=buy&buyid=" . $marketview['mID'] . "\">Buy</a><br /></td>";
                echo "</tr>";
            }
            echo "</table>";    
               
        }
        break;
 
        case "buy":
        {   
            $buy=stripslashes($_GET['buyid']);
            $query = $db->execute("SELECT m.Seller_ID, m.Price, m.Item_Sold_ID, m.market_ID, b.id as wid, b.name as wname from market_new m 
                                INNER JOIN blueprint_items b ON m.Item_Sold_ID = b.id
                                where m.market_ID=?", array($buy));
            $marketbuy = $query->fetchrow();
            if ($player->id == $marketbuy['Seller_ID']) 
            {
        
                echo "You can't buy your own items dummy<p>";
                echo "<a href=\"home.php\">Home</a>\n";
            }
            elseif($marketbuy['Price'] > $player->gold)
            {
                Echo "You don't have enough money to buy this item";
            }
            else
            {
                //update players info
                $itemcheck =  $db->execute("SELECT `player_id`, `item_id` FROM `items` WHERE `player_id`=? and `item_id`=?", array($player->id, $marketbuy['Item_Sold_ID']));
                $itemcheck1 = $itemcheck->fetchrow();
                $itembought=$marketbuy['Item_Sold_ID'];
                $seller=$marketbuy['Seller_ID'];
                if($itemcheck->recordcount() == 0)
                {
                
                    $db->query("INSERT INTO items (player_id,item_id,status,quantity) VALUES ('$player->id', '$itembought', 'unequipped', '1')");
                    $updatebuyergold = $db->execute("UPDATE `players` SET `gold` = `gold` - ? WHERE `id`=?", array($marketbuy['Price'], $player->id));
                    $updatesellergold = $db->execute("UPDATE `players` SET `gold` = `gold` + ? WHERE `id`=?", array($marketbuy['Price'], $marketbuy['Seller_ID']));
                    $deleteitem = $db->execute("delete from `market_new` where `market_ID`=?", array($marketbuy['market_ID']));
                    $logmsg = "Your\n" . $marketbuy['wname'] .  "\non the market was purchased by <a href=\"profile.php?id=" . $player->id . "\">" . $player->username . "</a> ";
                    addlog($marketbuy['Seller_ID'], $logmsg, $db);
                    echo "you just purchased this item from the market";
                }
                else
                {
             
                    $updateweapon = $db->execute("update `items` SET `quantity` = `quantity` + ? WHERE `player_id`=? and `item_id`=?", array(1, $player->id, $itembought));
                    $updatebuyergold = $db->execute("UPDATE `players` SET `gold` = `gold` - ? WHERE `id`=?", array($marketbuy['Price'], $player->id));
                    $updatesellergold = $db->execute("UPDATE `players` SET `gold` = `gold` + ? WHERE `id`=?", array($marketbuy['Price'], $marketbuy['Seller_ID']));
                    $deleteitem = $db->execute("delete from `market_new` where `market_ID`=?", array($marketbuy['market_ID']));
                    $logmsg = "Your\n" . $marketbuy['wname'] .  "\non the market was purchased by <a href=\"profile.php?id=" . $player->id . "\">" . $player->username . "</a> ";
                    addlog($marketbuy['Seller_ID'], $logmsg, $db);
                    echo "you just purchased this item from the market";
                }
                               
            }
 
        }
        break;
        case "sell":
        $sell=stripslashes($_GET['sellid']);
        {
            $itemcheck =  $db->execute("SELECT items.item_id, items.player_id, items.quantity, items.id, b.id, b.name FROM items 
                                       INNER JOIN blueprint_items b ON items.item_id = b.id
                                       WHERE items.player_id=? and items.item_id=?", array($player->id, $sell));
            $itemcheck1 = $itemcheck->fetchrow();
       
            if($itemcheck->recordcount() == 0)
            {
                echo "You don't own this item";
            }
            else
            {
                $item=$_GET['sellid'];
                echo "CAC BlackMarket if you would like to sell an item on the market our fee is 10% of the item sell price or at least $1.<br>This must be paid up front and is non refundable<p>"; 
                echo "You may only sell up to 3 items at one time and all items remain on the market for 5 days.<br>";
                echo "After 5 days the items will be put back into your inventory";
                echo "<form method=\"POST\" action=\"marketnew.php?act=confirm\" >";
                echo "You are placing your \n" . $itemcheck1['name'] . "\non the market.<p>"; 
                echo "<input type=\"hidden\" name=\"act\" value=\"confirm\">";
                echo "<input type=\"hidden\" name=\"item\" value=\"$item\">";
                echo "Asking Sell Price: <input type=\"text\" name=\"price\"><p>";
                echo "<input type=\"submit\" value=\"Submit\">";
                echo "</form>";
            }
            
        }
        break;
        case "confirm":
        {
            $item=stripslashes($_POST['item']);
            $price=stripslashes($_POST['price']);
            $fee=floor($price/10);
            if ($fee<1){$fee=1;}
            if($price<=0){
                echo "Sorry, We do not allow you to give things away.";
                break; 
            }else
            {
                echo   "Please confirm that you want to sell this item";
                echo "<form method=\"post\" action=\"marketnew.php?act=list\">";
                echo "<input type=\"hidden\" name=\"item\" value=\"$item\">";
                echo "<input type=\"hidden\" name=\"price\" value=\"$price\">";
                echo "<input type=\"hidden\" name=\"fee\" value=\"$fee\">";
                echo "<input type=\"submit\" name=\"list\" value=\"Yes, I am sure!\">";
                echo "</form>";
            }
        }
        break;
        case "list":
        {
            $item=stripslashes($_POST['item']);
            $price=stripslashes($_POST['price']);
            $total=$price;
            $fee=stripslashes($_POST['fee']);
            //check to see if player can afford to list
            if($player->gold<$fee)
            {   
                echo "Sorry, you can afford to list that item.";
            }
            elseif($item <= 0)
            {     
                echo "you must select an item to sell";
            }
            else
            {
                //add items to market 
                $additem = $db->execute("INSERT INTO market_new (Seller_ID,Price,Item_Sold_ID) VALUES ('$player->id', '$price', '$item')");
 
                //remove fee from player
                $query = $db->execute("update `players` set `gold`=? where `id`=?", array($player->gold - $fee, $player->id));
                        
                //remove item from inventory
                $gettotal =  $db->execute("SELECT `player_id`, `item_id`, `quantity` FROM `items` WHERE `player_id`=? and `item_id`=?", array($player->id, $item));
                $gettotal1 = $gettotal->fetchrow();
           
                if ($gettotal1['quantity'] > 1) 
                {
                    $update = $db->execute("UPDATE `items` set `quantity` = `quantity`- ? WHERE `player_id`=?", array(1, $player->id));    
                   
                    
                }
                else
                {
                    $remove = $db->execute("Delete from `items` where `player_id`=? and `item_id`=?", array($player->id, $item));
                } 
                echo "You have placed your item on the market";
            }
        }
        break;
        default: 
        {
            echo "Please Select a Market to search";
            echo "<table width=\"100%\" border=\"1\">";
            echo "<tr>";
            while ($categories = $getcategories->fetchrow())
            {
                echo "<td>";
                echo "<a href=\"marketnew.php?act=cat&catid=" . $categories['ID'] . "\">" .  $categories['Weapon_Type'] . "</a><br />";
                echo "</td>";
                echo "</tr>";
            }
            echo "</table>";  
        }
        break;
    }
    include("templates/private_footer.php"); 
?>
 

Re: Please take a look at my code

Posted: Tue Oct 28, 2008 8:19 pm
by requinix
Code looks fine, but you haven't posted everything so I can't say 100% sure it's safe.


There are two types of injection to look for: SQL and HTML.

There are three ways to protect against SQL injection. The first is to do it yourself using functions like mysql_real_escape_string: they take strings and escape (often with backslashes) them to they are safe to use in queries.

Code: Select all

"SELECT id FROM users WHERE name = '" . mysql_real_escape_string("Bill O'Reilly") . "'"
// SELECT id FROM users WHERE name = 'Bill O\\'Reilly'
Second is to validate the data itself. Regular expressions come in handy, but most people forget that PHP has a few functions you can use. For example, numbers can't have letters in them, there's only one decimal, you can have a negative sign in front, etc. Serial numbers have to be in a specific format, like 00AAAA00 (two numbers, four letters, two numbers). Point is, if you know exactly what is in the string and there's nothing you don't want, the data is inherently valid.
Third is to let something else do the work. That's what your code is doing, probably with prepared statements. As long as you put the ?s in the right places then you don't have to worry about anything.

HTML injection is easy: use htmlentities. It also works for XML and most anything else related.

Code: Select all

$user_input = "Bill <script>alert('XSS');</script>";
echo "<p>Hi $user_input</p>"; // "Hi Bill" and you see a dialog saying "XSS" - bad
echo "<p>Hi " . htmlentities($user_input) . "</p>"; // "Hi Bill <script>alert('XSS');</script>" and no dialog - good

Re: Please take a look at my code

Posted: Tue Oct 28, 2008 8:44 pm
by cdoyle
Thanks for replying,

So for the mysql_real_escape do I use this wherever there is user input?

For example, where the user can enter a price for the item they want to sell
echo "Asking Sell Price: <input type=\"text\" name=\"price\"><p>";

would I do something like this?
$price=stripslashes($_POST['price']);
mysql_real_escape_string($price);

Re: Please take a look at my code

Posted: Tue Oct 28, 2008 8:58 pm
by requinix
It's a function. It returns things. Just like stripslashes().

My advice is to use it only when putting stuff into queries, not before then.