Hi all,

I have the following code to process through my requests, then use in a database. FIRST of all, I was wondering if i have it in the right order, and SECOND, if there are any other steps i could do to secure up my application. have a look, use it if you like, and feel free to elaborate on any other security steps that could be used.
cheers team

$arr = array("submit", "itemID", "item_code", "description", "list_exc", "trade_exc", "rrp_exc", "length", "quantity", "lastUp");
foreach ($arr as $value)
    {
    $$value = $_REQUEST[$value] ;
    $$value = stripslashes($$value);
    $$value = mysql_real_escape_string($$value);
    }
    echo "now i write the item_code as + $item_code + and use it for example";

Recommended Answers

All 7 Replies

One question is $$value a valid variable?

Yes, it allows the variable to be used if register_globals is off. Another guy here on daniweb showed me it to fix a script one time, quite useful

Member Avatar for diafol

$$ can be useful, but I'd question its use as a general workaround. Security - NEVER use $_REQUEST!

$arr = array("submit", "itemID", "item_code", "description", "list_exc", "trade_exc", "rrp_exc", "length", "quantity", "lastUp");
$myarray = array();
foreach ($arr as $key){
    if(isset($_POST[$key]))$myarray[$key] = $_POST[$key] ;
}
function cleanMe($item){
    return mysql_real_escape_string(stripslashes($item));
}
$cleanarray = array_map("cleanMe", $myarray);

Now you have all the existing keys and values in an array. Now you do what you want with the clean data.
You can do an extract() to get all array keys as their own variables, but that's also a bit tricky as you're able to overwrite existing variables unless you set some sort of prefix. So IMO, it's much safer to satore all the data in an array and keep it there. The only difference for your echoing is that you have to use braces for array item variables:

echo "now i write the item_code as + {$cleanarray['item_code']} + and use it for example";

Hey bro (diafol), i've been waiting for you to respond lol, youre the man. Awesome advice from you as always. Are to able to elaborate on why post is safer than request? Its always been a wonder for me.

The original code i posted gets put into the form again eg, echoed in the value part of the input, that particular form was for adding items..

Here is the calendar form that allows entry, edit, delete of a calendar record. FYI, it is a backend invoicing / office application, and I use this structure style for every form regardless of if it is a customer item diary invoice etc. So i would change my code to the above code that you suggested, and this would be a security improvement? Or do you have any improvements other than the above that could improve security / functionality? i've added comments to try show what im getting everything to do

<?
// connect and start everything up like the page layout etc
session_start();
include("../config.php");
user();
connect();
    //get any requests 
$arr = array("cpt", "calID", "calo", "d", "cald", "calh", "calde", "calhe", "calt", "docID", "doctype", "call", "submit", "calinfo");
foreach ($arr as $value)
    {
    $$value = $_REQUEST[$value] ;
    $$value = stripslashes($$value);
    $$value = mysql_real_escape_string($$value);
    }
    // set the date for today
    $today = date("Y-m-d");
    // if the day aint today eg clicked from within a calender and there is no existing id
if(!$cald=="" && $calID=="")
{
$calde = $cald;
$calh = "09:00";
$calhe = "09:00";
}
// start a submit message
$MESSAGE = "";
// if calID is empty then set a new id and show a create button
if($calID=="")
{
$isnew = "true";
$calID = date("ymdHis");
}
// if a submit tbutton is clicked, then create update delete and show message
if($submit=="Create")
{
mysql_query("INSERT INTO `cal` (`cpt`, `calID`, `calt`, `call`, `cald`, `calh`, `calde`, `calhe`, `calo`, `calinfo`) VALUES ('$cpt', '$calID', '$calt', '$call', '$cald', '$calh', '$calde', '$calhe', '$calo', '$calinfo');");
$MESSAGE .= "$submit was successful";
 }
 if($submit=="Update")
 {
 mysql_query("UPDATE `cal` SET `cpt`= '$cpt', `calt` = '$calt', `call` = '$call', `cald` = '$cald', `calh` = '$calh', `calde` = '$calde', `calhe` = '$calhe', `calo` = '$calo', `calinfo` = '$calinfo' WHERE `calID` = '$calID'");
 $MESSAGE .="Updated successfully";
 }
 if($submit=="Delete")
 {
 mysql_query("DELETE FROM `cal` WHERE `calID` = '$calID'");
 $MESSAGE .="Deleted successfully";
 }
 // this brings in  values from other pages within the program
$calt = $_SESSION['calt'];
$call = $_SESSION['call'];
$calinfo = $_SESSION['calinfo'];
// if its not a new record then select it by the calID
 if(!$isnew=="true")
 {
 $excal = mysql_query("SELECT * FROM `cal` WHERE `calID` = '$calID'");
 while($xc = mysql_fetch_array($excal))
 {
 // loop through the items i want
 $arr = array("calo", "cald", "calh", "calde", "calhe", "calt", "call", "calinfo", "calID", "cpt");
foreach ($arr as $value)
    {
    $$value = $xc[$value] ;
    }
 }
 }
 // this isnt used any more, needs deleting
$expday = explode("-",$d);
$viey = $expday['0'];
$viem = $expday['1'];
$vied = $expday['2'];
// echo form with values in it, if its new the form is empty
echo"<h1><a href='/diary/day/$cald'>$cald</a></h1>
<p class='post-by'>$MESSAGE</p>";
echo"<form method='post' name='diary' action='/diary/$calID' name='diary' onsubmit='return check(this.name)'>
<input type='hidden' name='calID' value='$calID'><table id='fstable'>
<tr>
<td style='width:70px;' align='right'>
Title:</td><td><input type='text' name='calt' value='$calt' id='calt' style='width:100%;'></td><td align='right'>
Start:</td><td>
Day:<input type='date' name='cald' id='cald' value='$cald' onblur='weekday(this.value)' pattern='[0-9-0-9-0-9]{10}' title='YYYY-MM-DD' placeholder='".date("Y-m-d")."' style='width:130px;'>
Time:<input type='time' name='calh' value='$calh' id='calh' style='width:110px;' title='HH:MM' /><span id='weekday'></span>   <a onclick='diacop()'>Start = End</a></td>
</tr>
<tr><td align='right'><a href='http://maps.google.co.nz/maps?q=$call' target='_blank'>Location:</a></td><td><input type='text' name='call' style='width:100%;' id='call' value='$call'></td>
<td align='right'>
End:</td><td>
Day:<input type='date' name='calde' id='calde' value='$calde' onblur='weekday(this.value)' pattern='[0-9-0-9-0-9]{10}' title='YYYY-MM-DD' style='width:130px;'>
Time:<input type='time' name='calhe' value='$calhe' id='calhe' style='width:110px;' title='HH:MM' />";
switch ($cpt)
{
case yes:
 $switch = "no";
  break;
case no:
  $switch = "yes";
  break;
default:
$switch = "yes";

}
echo" Completed:<select name='cpt' id='cpt'>
<option value='$cpt'>$cpt</option>
<option value='$switch'>$switch</option>
</select></td>
</tr>
";


echo"<tr><td align='right'>Info:</td><td colspan='3'><textarea name='calinfo' style='width:100%;height:200px;' id='calinfo'>$calinfo</textarea></td></tr>
<tr><td colspan='4' align='center'>";
// if new then show create
if($isnew=="true")
{
echo"   <input type='submit' name='submit' value='Create'>  ";
}
echo"<input type='submit' name='submit' value='Update'> <input type='submit' name='submit' value='Delete'>";

echo"</td></tr></table></form>";
disconnect();
?>

Disclaimer! A well written form processor class can survive pretty much all of the exploits and tricks that the hackers use. NOT ALL THOUGH...

Hi,

$_REQUEST is one dangerous thing if you don't protect your script pretty well.. I mean extremely well. There have been debates and many disagreements about this. One might argue, "why did they put it there?", and "Why not just eliminate it all in all?".

The response to this is simple. Say, we have an application where security is not that much of a concern, because there is nothing there to protect like payment system or login system where cookie is not present or not being implemented. This type of applications will get the full benefits of request, because the coder don't even have to assign method on their form tags. A classic example is a simple photogallery, where clients can click on the thumbnail and take them to a larger version of the photo.. you can either use get or request on this one. Another example is a pagination script with limited database interaction...

However, using it in a system where there is a log in, a cart system that heavily relies on cookies.. Using the $_REQUEST can increase the risks for your clients.

This response is going to be lengthy, if I will try to explain it in great detail. However, here is an article about it please read it here. I agree with this person 90.00%.

Learning how to program is like learning another language, and the most exciting about learning a new language is learning the bad words at first, and then the good one. One bad thing I have learned before actually learning how to code in php was how to break-in in an applications written in PHP. Breaking-in in an application is not that hard. Yahoo php community ONCE CALLED ME MORPHEUS LINUX.:) .( that was when I was a curious, but stupid 15 years old bored high school student). Most applications has its own weaknesses and exploits or rather a hole where the exploits can be introduce. It could be either by way of remote form submission or just a simple mysql error that the application's author just ignore to suppressed.

Now, let's get back to your $_REQUEST.. let say I have been visiting your site, I would look into the source codes of your form. The first thing I would do is send a simple cURL submission to your site. Of course, I need to see what is your form processors document e.g. index.php.

Ok, at this time I already stablished my target.. and I suspect you are probably using $_REQUEST ,I prefer this because of the ease of sending data even on the browser. The second observation I have to make is to test if base on your $_REQUEST results I will be able to see how your products are acquired from the mysql... Believe me, I have not been in daniweb for a long time, but I can honestly tell you there are many of these codes posted here, showing the same vulnerabilities as what I am about to discuss.

vulnerability one, this query is no good when getting the product from database..

SELECT* FROM product_table WHERE id = '$_REQUEST['id']';

Now, if you are not carefull or don't even bother protecting your codes, I can easily break in just by typing this on my browser. The codes about should be fully sanitized first.

  http://YourDomainDotCom/index.php?id = | OR | = |

So, Will this work? yes becaus request will work on either PoSt or GET., you just made my life a lot easier..the original database query above will convert to something like this..

 SELECT* FROM product_table WHERE id = '| OR | = |';

Guess, what is going to happen once the above query has been executed.. Try it on your xampp and see what will happen///...

I must stop here..., because this is not a php hacking and vulnerability forum. In fact, this is a community where other people help other people..

Regardless of $_GET, $_REQUEST, or $_POST.. if you are careful on what you allow your user to feed your form processor.. , the risks and vulnerabilities can be minimized.

Again just a friendly reminder, as a site owner/developer, there are responsibilities placed on our shoulders,,, and one of them is protecting sensitive information submitted by our clients.

I don't want to give too much information here due to my fear of an overnight increase of hack wannabies.., and hopefully they are not all coming from daniweb... :),

looking at my code above though, is it safe in general? any thoughts?

Try searching for mysql sanitation on google.. like this one here.

Protect your site from all data coming from the outside..

The rule of thumb is to sanitize it before sending it your database. ONLY advance users can do the shell exploitation. More likely, advance users are always busy with either work or schooling, so I don't see these group of people going out there to be hacking all the sites they see, unless they are insulted or challenged. The dangerous ones are the ones who are currently experimenting and trying to prove something to their friends or just for the fun of it.

Honestly, even with all the sanitation filters we have, some people can still be successful in exploiting the site using $_GET or $_REQUEST... NO! $_POST is not exempted (just different ways of doing it).

Even with all the sanitation filters applied to form input, these codes can pass through without problem

 login.php?id=1+AND+extractvalue(1,concat(0x5c,(select pass from users limit 0,1)))
 ## I have to remove the main content of the exploit to prevent people from using it.

What the exploit above will do is to echo the password from user. Why can this go trhough even with extensive santition? Take a look at the characters of the exploit, they are all valid.. it can easily give away password. This can be easily done on GET and REQUEST..

The danger can even escalate if people are careless or just too lazy to clean up their form collected data.. Please take a look at my sample mother of all exploit below..

  ## the most dangerous one
index.php?id=1+union+select+'<?php($_request[shell]);?>'+into+outfile+'/www/somedirectoryIwant/bad.php'

Gues what is going to happen next, I just type in the url of the bad.php and bingo things are given to me for free.

Things we can do the least.

  1. Name your table columns differently. If possible, some terms that only you would know.
  2. Avoid using "pass", "password" "productID" "user" "username" as database column names. This type of naming convention is a complete give away or an inventation to get exploited.

  3. If using an auto increment as ID , try setting it up for larger numbers or triple digits. Normally, mysqly would start counting from 1.. If this is the case, then take a look at one of the exploit sample above..

  4. Check your server's directory everyday, and make sure there is no unfamiliar directory or files residing in there. This is common if you are allowing a file uploads in your server. Make sure onlly the allowed extensions are being saved in your server.

  5. Learn how to suppressed mysql query errors. If you allow this to be showing on your page.. make sure the error is minimal and without any wrong password, wrong database username showing on your page .e.g 'cannot connect to database using someUser with password = true...

  6. IMPORTANT! try using PDO or mysqli (these has a built security in them already).

  7. Use pHP framework (CI,Zend,or CAKE), and at the least use templating system like smarty, twig, and others.. ( templating system like smarty are compiled, so it gives a little buffer ).

  8. One last thing if you will be using $_GET, use filler on the url, instead of just going for something like this index.php?id=2 . This is pretty vulnerable to exploitation. Do it like this index.php?id=2&filler= 1665e9sdf49we4ga+6s8e9ee9g4s6s4g6s89ww4s6g4s68e974g6s&someotherstuffs=myStuff&anotherFiller= alqoeis4se876dwwhh4e9r8r9h4h64s99h4s699e94h6s64h. The objective here to get closer as much as possible to 2000 bytes. By implementing this there are only few characters left before the limit. Take a look on how microsoft and google for example. They would literray filled the entire address bar...

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.