Some help tidying/securing this PHP/SQL pleaseeee :)

Associate
Joined
23 Mar 2009
Posts
348
Location
Midlands
Hey all,

I'm quite new to PHP and SQL and wrote this script for my work (for free) by reading tutorials and the like. I was just wondering if one of you could be kind enough to have a read through and point out any noticeable things I should change please? :)

It's a basic thing to log special orders from a customer so my other staff can look them up should a customer enquire about it's progress.
The header.php is just styling and the database login details.

index.php
PHP:
<?php include("template/header.php"); ?>
<div id=header>View current orders</div>
<p>To view details of an order, just click on the order number.<br />To edit details of an order, just click on one of the other fields</p>
<div id=main>Please select an order number from the list below:</div>
<BLOCKQUOTE><div align=center><table border="1">
<tr>
<td><a href="index.php?OrderBy=OrderNum">Order ID</a></td>
<td><a href="index.php?OrderBy=CustName">Customer Name</a></td>
<td><a href="index.php?OrderBy=CustNum1">Contact Number</a></td>
<td><a href="index.php?OrderBy=OrderDate">Order Date</a></td>
<td><a href="index.php?OrderBy=OrderStatus">Order Status</a></td>
<tr><td>
<?php
$OrderBy = $_GET['OrderBy'];
if (!isset($OrderBy)) {
    $OrderBy = "OrderNum";
}
  $result = mysql_query(
            "SELECT OrderNum FROM specials ORDER BY $OrderBy");
  if (!$result) {
    echo("<P>Error performing query: " .
         mysql_error() . "</P>");
    exit();
  }

  while ( $row = mysql_fetch_array($result) ) {
$hehe = $row["OrderNum"];
    echo("<a href='view.php?id=" . $row["OrderNum"] . "'>"  . $row["OrderNum"] . "</a></br>");
  }
?>
</td><td>
<?php
  $result = mysql_query(
            "SELECT CustName FROM specials ORDER BY $OrderBy");
  if (!$result) {
    echo("<P>Error performing query: " .
         mysql_error() . "</P>");
    exit();
  }
  while ( $row = mysql_fetch_array($result) ) {
$hehe = $row["CustName"];
    echo("<a href='editbyname.php?id=" . $row["CustName"] . "'>" . $row["CustName"] . "</a></br>");
  }
?></td><td>
<?php
  $result = mysql_query(
            "SELECT CustNum1 FROM specials ORDER BY $OrderBy");
  if (!$result) {
    echo("<P>Error performing query: " .
         mysql_error() . "</P>");
    exit();
  }
  while ( $row = mysql_fetch_array($result) ) {
$hehe = $row["CustNum1"];
    echo($row["CustNum1"] . "</br>");
  }
?></td><td>
<?php
  $result = mysql_query(
            "SELECT OrderDate FROM specials ORDER BY $OrderBy");
  if (!$result) {
    echo("<P>Error performing query: " .
         mysql_error() . "</P>");
    exit();
  }
  while ( $row = mysql_fetch_array($result) ) {
$hehe = $row["OrderDate"];
    echo($row["OrderDate"] . "</br>");
  }
?></td><td>
<?php
  $result = mysql_query(
            "SELECT OrderStatus FROM specials ORDER BY $OrderBy");
  if (!$result) {
    echo("<P>Error performing query: " .
         mysql_error() . "</P>");
    exit();
  }
  while ( $row = mysql_fetch_array($result) ) {
$hehe = $row["OrderStatus"];
    echo($row["OrderStatus"] . "</br>");
  }
?>
</td></tr></table></div>
</BLOCKQUOTE>
<?php
  echo "<a href='addnew.php'>Add a new order</a><br />";
  echo "<a href='javascript:location.reload(true)'>Refresh this page</a>"; 
?>
<?php include("template/footer.php"); ?>
</BODY>
</HTML>

addnew.php
PHP:
<?php include("template/header.php"); ?>
<div id=header>Add a new order</div>
<p>To add an order, please fill out the below form and press "submit".</p>
<?php

if (isset($_POST['submit'])):
  $dbcnx = mysql_connect('localhost', '<<REMOVED>>', '<<REMOVED>>');
  mysql_select_db('database');

  $OrderNum = $_POST['OrderNum'];
  $OrderDate = $_POST['OrderDate'];
  $CustName = $_POST['CustName'];
  $CustAdd1 = $_POST['CustAdd1'];
  $CustPostCode = $_POST['CustPostCode'];
  $CustNum1 = $_POST['CustNum1'];
  $CustNum2 = $_POST['CustNum2'];
  $OrderStatus = $_POST['OrderStatus'];
  $sql = "INSERT INTO specials SET ID = '' , OrderNum='$OrderNum', OrderDate='$OrderDate' , CustName='$CustName' , CustAdd1='$CustAdd1' , CustPostCode='$CustPostCode' , CustNum1='$CustNum1' , CustNum2='$CustNum2' , OrderStatus='$OrderStatus'";
          
  if (@mysql_query($sql)) {
    echo('<p>New order added</p>');
  } else {
    echo('<p>Error adding new order: ' . mysql_error() . '</p>');
  }
?>

<p><a href="<?=$_SERVER['PHP_SELF']?>">Add order</a></p>

<?php
  else: // Allow the user to enter a new order
?>
<form action="addnew.php?addnew=yes" method="post">
<p>Enter the new order:<br />
Order Number: <input type="text" name="OrderNum" size="20" maxlength="255" /><br />
Order Date: <input onclick="ds_sh(this);" type="text" name="OrderDate" size="20" maxlength="20" /><br />
Customer Name: <input type="text" name="CustName" size="20" maxlength="50" /><br />
Customer Address: <input type="text" name="CustAdd1" size="20" maxlength="255" /><br />
Customer Postal Code: <input type="text" name="CustPostCode" size="7" maxlength="7" /><br />
Customer Home Phone: <input type="text" name="CustNum1" size="13" maxlength="13" /><br />
Customer Mobile: <input type="text" name="CustNum2" size="13" maxlength="13" /><br />
Customer Order Status: <input type="text" name="OrderStatus" size="20" maxlength="255" value="New" /><br />
<input type="submit" name="submit" value="SUBMIT" /></p>
</form>

<?php endif; ?>
<?php
  echo "<a href='index.php'>View current orders</a>"; 
?>
<?php include("template/footer.php"); ?>
</body>
</html>

view.php
PHP:
<?php include("template/header.php"); ?>
<div id=header>View an existing order</div>
<p>This will display an existing order. Use the links at the bottom of the page to select what you want to do.</p>
<?php
  $dbcnx = mysql_connect('localhost', '<<REMOVED>>', '<<REMOVED>>');
  mysql_select_db('database');

  $OrderNum = $_POST['OrderNum'];
  $OrderDate = $_POST['OrderDate'];
  $CustName = $_POST['CustName'];
  $CustAdd1 = $_POST['CustAdd1'];
  $CustPostCode = $_POST['CustPostCode'];
  $CustNum1 = $_POST['CustNum1'];
  $CustNum2 = $_POST['CustNum2'];
  $OrderStatus = $_POST['OrderStatus'];

$id = $_GET["id"];
$result = mysql_query("SELECT * FROM specials WHERE OrderNum=$id") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$onumber = $row['OrderNum'];
$result = mysql_query("SELECT * FROM specials WHERE OrderNum=$id") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$odate = $row['OrderDate'];
$result = mysql_query("SELECT * FROM specials WHERE OrderNum=$id") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cname = $row['CustName'];
$result = mysql_query("SELECT * FROM specials WHERE OrderNum=$id") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cadd1 = $row['CustAdd1'];
$result = mysql_query("SELECT * FROM specials WHERE OrderNum=$id") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cpc = $row['CustPostCode'];
$result = mysql_query("SELECT * FROM specials WHERE OrderNum=$id") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cnum1 = $row['CustNum1'];
$result = mysql_query("SELECT * FROM specials WHERE OrderNum=$id") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cmob = $row['CustNum2'];
$result = mysql_query("SELECT * FROM specials WHERE OrderNum=$id") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$ostatus = $row['OrderStatus'];
?>

Order Number: <b><?php echo($onumber); ?></b><br />
Order Date: <?php echo($odate); ?><br />
Customer Name: <b><?php echo($cname); ?></b><br />
Customer Address: <?php echo($cadd1); ?><br />
Customer Postal Code: <?php echo($cpc); ?><br />
Customer Home Phone: <?php echo($cnum1); ?><br />
Customer Mobile: <?php echo($cmob); ?><br />
Customer Order Status: <?php echo($ostatus); ?><br />
<br />
<?php
  echo "<a href='addnew.php'>Add new order</a></br>"; 
  echo "<a href='editbyordernum.php?id=" . $id . "'>Edit this order</a></br>"; 
  echo "<a href='javaScript:window.print();'>Print this order</a></br>";
  echo "<a href='index.php'>View current orders</a></br>"; 
  echo "<a href='delete.php?id=" . $id . "'>Delete this order</a>";
?>
<?php include("template/footer.php"); ?>
</body>
</html>

and lastly editbyname.php
PHP:
<?php include("template/header.php"); ?>
<div id=header>Edit an existing order</div>
<p>To edit an order, please change the values below and press "submit".</p>
<?php

if (isset($_POST['submit'])):
  $dbcnx = mysql_connect('localhost', '<<REMOVED>>', '<<REMOVED>>');
  mysql_select_db('database');

  $OrderNum = $_POST['OrderNum'];
  $OrderDate = $_POST['OrderDate'];
  $CustName = $_POST['CustName'];
  $CustAdd1 = $_POST['CustAdd1'];
  $CustPostCode = $_POST['CustPostCode'];
  $CustNum1 = $_POST['CustNum1'];
  $CustNum2 = $_POST['CustNum2'];
  $OrderStatus = $_POST['OrderStatus'];
  $del = mysql_query("DELETE FROM specials WHERE OrderNum = $OrderNum") or die(mysql_error());
  $sql = "INSERT INTO specials SET ID = '' , OrderNum='$OrderNum', OrderDate='$OrderDate' , CustName='$CustName' , CustAdd1='$CustAdd1' , CustPostCode='$CustPostCode' , CustNum1='$CustNum1' , CustNum2='$CustNum2' , OrderStatus='$OrderStatus'";
          
  if (@mysql_query($sql)) {
    echo('<p>Order updated!</p>');
  } else {
    echo('<p>Error updating order: ' . mysql_error() . '</p>');
  }

  else: // Allow the user to enter a new order

$id = $_GET["id"];
$result = mysql_query("SELECT * FROM specials WHERE CustName='$id'") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$onumber = $row['OrderNum'];
$result = mysql_query("SELECT * FROM specials WHERE CustName='$id'") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$odate = $row['OrderDate'];
$result = mysql_query("SELECT * FROM specials WHERE CustName='$id'") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cname = $row['CustName'];
$result = mysql_query("SELECT * FROM specials WHERE CustName='$id'") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cadd1 = $row['CustAdd1'];
$result = mysql_query("SELECT * FROM specials WHERE CustName='$id'") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cpc = $row['CustPostCode'];
$result = mysql_query("SELECT * FROM specials WHERE CustName='$id'") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cnum1 = $row['CustNum1'];
$result = mysql_query("SELECT * FROM specials WHERE CustName='$id'") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$cmob = $row['CustNum2'];
$result = mysql_query("SELECT * FROM specials WHERE CustName='$id'") or die(mysql_error()); 
while($row = mysql_fetch_array($result))
$ostatus = $row['OrderStatus'];
?>

<form action="editbyname.php" method="post">
<p>Enter the new order:<br />
Order Number: <input type="text" name="OrderNum" size="20" maxlength="255" value='<?php echo($onumber); ?>' /><br />
Order Date: <input onclick="ds_sh(this);" type="text" name="OrderDate" size="20" maxlength="20" value='<?php echo($odate); ?>' /><br />
Customer Name: <input type="text" name="CustName" size="20" maxlength="50" value='<?php echo($cname); ?>' /><br />
Customer Address: <input type="text" name="CustAdd1" size="20" maxlength="255" value='<?php echo($cadd1); ?>' /><br />
Customer Postal Code: <input type="text" name="CustPostCode" size="7" maxlength="7" value='<?php echo($cpc); ?>' /><br />
Customer Home Phone: <input type="text" name="CustNum1" size="13" maxlength="13" value='<?php echo($cnum1); ?>' /><br />
Customer Mobile: <input type="text" name="CustNum2" size="13" maxlength="13" value='<?php echo($cmob); ?>' /><br />
Customer Order Status: <input type="text" name="OrderStatus" size="20" maxlength="255" value='<?php echo($ostatus); ?>' /><br />
<input type="submit" name="submit" value="SUBMIT" /></p>
</form>

<?php endif; ?>
<?php
  echo "<a href='addnew.php'>Add new order</a></br>"; 
  echo "<a href='index.php'>View current orders</a></br>"; 
  echo "<a href='delete.php?id=" . $id . "'>Delete this order</a>";
?>
<?php include("template/footer.php"); ?>
</body>
</html>

All help would be greatly appreciated :)
It all seems to work (I've installed XAMPP locally to test it). I'm not too worried about how secure it is, but more how retard-proof it is :p
 
Suggest you look into using parameterized queries. As to why - google SQL Injection Attacks. As far as being retard proof is concerned.. the only thing that will prove this is testing it with users. Difficult to say by looking at the code.
 
^ Definitely need to look into SQL injection prevention. If I modified your index.php to read something like:

PHP:
index.php?OrderBy=OrderStatus;Drop+table+specials--

index.php would execute:
PHP:
SELECT OrderNum FROM specials ORDER BY OrderStatus;Drop table specials--
(-- is a comment; so any SQL following my input would be ignored to improve my chances of it working)

and the specials table would probably be deleted :p.


A quick way to get around it would be to check that the order by variable is a legal parameter, or use switch to force a good value no matter what. I.e. :

PHP:
switch ( $_GET["orderby"] ) {
	case "OrderNum":
		$OrderBy = "OrderNum";
	break;
	case "CustName":
		$OrderBy = "CustName";
	break;
	default:
		$OrderBy = "OrderNum";
	break;
}
 
^ Definitely need to look into SQL injection prevention. If I modified your index.php to read something like:

PHP:
index.php?OrderBy=OrderStatus;Drop+table+specials

index.php would execute:
PHP:
SELECT OrderNum FROM specials ORDER BY OrderStatus;Drop table specials

and the specials table would probably be deleted :p.


A quick way to get around it would be to check that the order by variable is a legal parameter, or use switch to force a good value no matter what. I.e. :

PHP:
switch ( $_GET["orderby"] ) {
	case "OrderNum":
		$OrderBy = "OrderNum";
	break;
	case "CustName":
		$OrderBy = "CustName";
	break;
	default:
		$OrderBy = "OrderNum";
	break;
}
Only if register globals is enabled, which it is not by default.
 
Code is above mate, just need to validate the input rather than relying on the url to provide a valid string for 'OrderBy'. Just take the $_GET['OrderBy'] and check what it contains and make sure that it is one of the values that you want. If it isnt then set the $orderby variable to the most common order by value.
 
Back
Top Bottom