would this work??

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
Ralle
Forum Commoner
Posts: 38
Joined: Mon Oct 17, 2005 5:05 am

would this work??

Post by Ralle »

hello!
Would this script do any form of securing?

Code: Select all

$name = $_POST[sitename];
$url = $_POST[siteurl];
$desc = $_POST[sitedesc];
$cat = $_POST[sitecat];
$lang = $_POST[sitelang];

function sqlclean ($string) 
{ 
    if (get_magic_quotes_gpc()) { 
        $string = stripslashes($string); 
    } 
    return mysql_real_escape_string($string); 
} 

sqlclean($name);
sqlclean($siteurl);
sqlclean($sitedesc);
sqlclean($sitecat);
sqlclean($sitelang);
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

only if you were already connected to MySQL.

quote your array indices

Code: Select all

$lang = $_POST['sitelang'];
User avatar
shoebappa
Forum Contributor
Posts: 158
Joined: Mon Jul 11, 2005 9:14 pm
Location: Norfolk, VA

Post by shoebappa »

In general I would do whatever checking before moving the post data to a variable. Although to be honest I don't think running the function you had would actually do anything to clean the variable. Meaning the function may work, but the way you called it wouldn't do anything. You're calling the function but not storing what the function returns.

Something like:

Code: Select all

function sqlclean ($string) 
{ 
    if (get_magic_quotes_gpc()) { 
        $string = stripslashes($string); 
    } 
    return mysql_real_escape_string($string); 
} 

// You changed variable names, I probably wouldn't, for the very reason you showed above.  You stored the post values in names without "site" and then later called the functions (all but the name one) with "site" in front.  This would cause problems down the road, which is clean and which isn't?
$sitename = sqlclean($_POST["sitename"]); 
$siteurl = sqlclean($_POST["siteurl"]); 
$sitedesc = sqlclean($_POST["sitedesc"]); 
$sitecat = sqlclean($_POST["sitecat"]); 
$sitelang = sqlclean($_POST["sitelang"]);
User avatar
shoebappa
Forum Contributor
Posts: 158
Joined: Mon Jul 11, 2005 9:14 pm
Location: Norfolk, VA

Post by shoebappa »

I mentioned a question in a comment above.. Which is clean and which isn't? I've also seen an array used to store cleaned values.

So:

Code: Select all

function sqlclean ($string) 
{ 
    if (get_magic_quotes_gpc()) { 
        $string = stripslashes($string); 
    } 
    return mysql_real_escape_string($string); 
} 

$clean["sitename"] = sqlclean($_POST["sitename"]); 
$clean["siteurl"] = sqlclean($_POST["siteurl"]); 
$clean["sitedesc"] = sqlclean($_POST["sitedesc"]); 
$clean["sitecat"] = sqlclean($_POST["sitecat"]); 
$clean["sitelang"] = sqlclean($_POST["sitelang"]);
I don't see a tremendous ammount of value there but I guess you could probably do it this way in a loop or something. That and you know that you cleaned it if it's in the clean array...
Ralle
Forum Commoner
Posts: 38
Joined: Mon Oct 17, 2005 5:05 am

Post by Ralle »

which is clean and which isn't?
huh??
I learned something about securing the strings there.
User avatar
shoebappa
Forum Contributor
Posts: 158
Joined: Mon Jul 11, 2005 9:14 pm
Location: Norfolk, VA

Post by shoebappa »

See your code...

Code: Select all

$name = $_POST[sitename]; 
$url = $_POST[siteurl]; 
$desc = $_POST[sitedesc]; 
$cat = $_POST[sitecat]; 
$lang = $_POST[sitelang]; 

function sqlclean ($string) 
{ 
    if (get_magic_quotes_gpc()) { 
        $string = stripslashes($string); 
    } 
    return mysql_real_escape_string($string); 
} 

sqlclean($name); 
sqlclean($siteurl); 
sqlclean($sitedesc); 
sqlclean($sitecat); 
sqlclean($sitelang);
Lets pretend you store what the function returns back in the variable. You moved the post data into $name, $url, $desc, $cat, and $lang. But then called the funtion on $name, $siteurl, $sitedesc, $sitecat, and $sitelang...

So for the last four, your variables are all confused. I was just pointing out that it'd be hard to tell what you cleaned and what you didn't. Depending on the php configuration, the $_Post data may not automatically be stored in variable names. So since you stored $_POST["siteurl"] in $url, and then cleaned $siteurl, you ran the function on a variable that didn't exist yet, so it had no value when the function was called.

So later on when you try to use those variables you don't know what's what... The code I posted above would eliminate that confusion.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Three points, two of which have already been covered :)

1. You need to refer to array indices within the context of what type the key is, in this example, they are strings, thus need to be referred to as strings, but using quotes.

2. You need to set a variable for storing the return value, e.g. $name = sqlclean($name);

3. You need to connect to MySQL before using mysql_real_escape_string(), so just ensure you have you script in a similar order to the following:

Code: Select all

<?php

function sqlclean ($string)
{
    if (get_magic_quotes_gpc()) {
        $string = stripslashes($string);
    }
    return mysql_real_escape_string($string);
}

mysql_connect('yourhost', 'yourusername', 'yourpassword') or die ('Error connecting to DB!');
mysql_select_db('yourdb') or die('Error selecting DB!');

$name = sqlclean($_POST['sitename']);
$url = sqlclean($_POST['siteurl']);
$desc = sqlclean($_POST['sitedesc']);
$cat = sqlclean($_POST['sitecat']);
$lang = sqlclean($_POST['sitelang']); 

?>
HTH :)

And it's nice to see someone use my snippet :P
Last edited by Jenk on Mon Oct 24, 2005 6:17 am, edited 1 time in total.
Ralle
Forum Commoner
Posts: 38
Joined: Mon Oct 17, 2005 5:05 am

Post by Ralle »

it's just great.. I got it up and working.

So... is my script fully secure when every input goes through this function?? or are there anything else I need to know??
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

you have "reasonable" security.
Ralle
Forum Commoner
Posts: 38
Joined: Mon Oct 17, 2005 5:05 am

Post by Ralle »

what could be done to make it even safer?? will people be able to access my database with that input?
Post Reply