security isues

Ask about a PHP problem here.
Post Reply
User avatar
ta2shop
Posts: 179
Joined: Sat May 07, 2011 9:07 am
Location: madrid, Spain
Contact:

security isues

Post by ta2shop »

hy guys, i just finised the tutorial alex made about a mini shoping cart, and since the last one was.... well bad, i was hoping you can givet a lock, and mabye help my improve the security in it.
i want to add every posible thing but i want to make it to slow in the browser!
can you suggest somthing?
thanks
cart.php
[syntax=php]
<?php
session_start();

$page = "index.php";

mysql_connect("localhost","root","") or die(mysql_error());
mysql_select_db("ta2cart") or die(mysql_error());

if(isset($_GET['add'])) {
$quantity = mysql_query('SELECT id, quantity FROM products WHERE id='.mysql_real_escape_string((int)$_GET['add'])) or die(mysql_error());
while($quantity_row = mysql_fetch_assoc($quantity)) {
if($quantity_row['quantity']!=$_SESSION['cart_'.(int)$_GET['add']]) {
$_SESSION['cart_'.(int)$_GET['add']]+='1';
}
}
header('Location: '.$page);
}

if(isset($_GET['remove'])) {
$_SESSION['cart_'.(int)$_GET['remove']]--;
header('Location: '.$page);
}
if(isset($_GET['delete'])) {
$_SESSION['cart_'.(int)$_GET['delete']]='0';
header('Location: '.$page);
}

function products() {
$get = mysql_query('SELECT id, name, description, price FROM products WHERE quantity > 0 ORDER BY id DESC') or die(mysql_error());
if(mysql_num_rows($get)==0) {
echo "There are no products to display!";
}
else {
while($get_row = mysql_fetch_assoc($get)) {
echo'<p><b>'.$get_row['name'].'</b><br/>'.$get_row['description'].'<br/>'.number_format($get_row['price'],2).'&euro; <a href="cart.php?add='.$get_row['id'].'">Add</a><p/>';
}
}
}

function paypal_items() {
$num = 0;
foreach($_SESSION as $name => $value) {
if($value!=0) {
if(substr($name,0 ,5)=='cart_') {
$id = substr($name, 5, strlen($name)-5);
$get = mysql_query('SELECT id, name, price, shipping FROM products WHERE id='.mysql_real_escape_string((int)$id)) or die(mysql_error());
while($get_row = mysql_fetch_assoc($get)) {
$num++;
echo '<input type="hidden" name="item_name_'.$num.'" value="'.$get_row['name'].'">';
echo '<input type="hidden" name="item_number_'.$num.'" value="'.$id.'">';
echo '<input type="hidden" name="amount_'.$num.'" value="'.$get_row['price'].'">';
echo '<input type="hidden" name="shipping_'.$num.'" value="'.$get_row['shipping'].'">';
echo '<input type="hidden" name="shipping2_'.$num.'" value="'.$get_row['shipping'].'">';
echo '<input type="hidden" name="quantity_'.$num.'" value="'.$value.'">';
}
}
}
}
}

function cart() {
foreach($_SESSION as $name => $value) {
if($value>0) {
if(substr($name, 0, 5)=='cart_') {
$id = substr($name, 5, (strlen($name)-5));
$get = mysql_query('SELECT id, name, price FROM products WHERE id='.mysql_real_escape_string((int)$id));
while($get_row = mysql_fetch_assoc($get)) {
$sub = $get_row['price']*$value;
echo $get_row['name'].' x '.$value.' @ '.number_format($get_row['price'], 2).'&euro; = '.number_format($sub, 2).'&euro;'.'
<a href=\'cart.php?remove='.$id.'\'>[-]</a>
<a href=\'cart.php?add='.$id.'\'>[+]</a>
<a href=\'cart.php?delete='.$id.'\'>[Delete]</a><br/>';
}
}
$total += $sub;
}
}
if($total ==0) {
echo"Your cart is empty.";
}
else {
echo'<br/> Total: '.number_format($total, 2).'&euro;';
?>
<p>
<form action="https://www.paypal.com/cgi-bin/webscr" method="post">
<input type="hidden" name="cmd" value="_cart">
<input type="hidden" name="upload" value="1">
<input type="hidden" name="business" value="paypal@ta2shop.es">
<?php paypal_items(); ?>
<input type="hidden" name="currency_code" value="EUR">
<input type="hidden" name="amount" value="<?php echo $total; ?>">
<input type="image" src="http://www.paypal.com/es_XC/i/btn/x-click-but03.gif" name="submit" alt="Realice pagos con PayPal: es rápido, sin costo y seguro">
</form>
</p>
<?php
}
}

?>
[/syntax]
Image
User avatar
jacek
Site Admin
Posts: 3262
Joined: Thu May 05, 2011 1:45 pm
Location: UK
Contact:

Re: security isues

Post by jacek »

moved to php security ;)

Nothing looks like it would be sql injectable to me

there is no need to use mysql_real_escape_string and (int) though so

[syntax=php]mysql_real_escape_string((int)$_GET['add'])[/syntax]
can be

[syntax=php](int)$_GET['add'][/syntax]
Which applies to all the other similar cases.

Depending on who is allowed to set product descriptions you may want to use htmlentities on things like $get_row['description'].

Overall looks fine.
Image
User avatar
ta2shop
Posts: 179
Joined: Sat May 07, 2011 9:07 am
Location: madrid, Spain
Contact:

Re: security isues

Post by ta2shop »

well i am the only one with back end permisions, and i just folowd the tutorial, if you say it is good to remouve the mysql_real_escape_string, i will ;)
Image
Post Reply