0

So, I've got some pretty basic code that I can't get to work quite right. I'm using the ezSQL class (http://justinvincent.com/ezsql), but that's working fine. Everything works fine, except when I try to use a sanitize function (get_post). I'm using two other functions, sanitizeString and sanitizeMySQL when I call the function get_post. If I just $_POST the data right to the SQL table, it works fine. It's only when I go thru the post function that it breaks. Here's the posting bit:

if (isset($_POST['username']) &&
isset($_POST['password']))
{
    $username = get_post('username');
    $password = get_post('password');
    $db->query("INSERT INTO users VALUES ('$username', '$password')");
}

Like I said, I'm using a class (that's what the $db->query stuff is about), but that's working perfectly. If I change the code to this:

if (isset($_POST['username']) &&
isset($_POST['password']))
{
    $username = $_POST['username'];
    $password = $_POST['password'];
    $db->query("INSERT INTO users VALUES ('$username', '$password')");
}

it works fine. Here are the three functions I'm using for sanitizing:

 // Sanitize Functions
function sanitizeString($var)
{
 if (get_magic_quotes_gpc()) $var = stripslashes($var);
 $var = htmlentities($var);
 $var = strip_tags($var);
 return $var;
}    

function sanitizeMySQL($var)
{
 $var = mysql_real_escape_string($var);
 $var = sanitizeString($var);
 return $var;
}    

function get_post($var) 
{
return sanitizeMySQL($_POST['$var']);
}

I've even tried just changing the get_post function to contain a mysql_real_escape_string return, and even that doesn't work. Also, I guess I should clarify what actually happens when I try to use the get_post function. It appears to create a new row in the table, but with completely empty cells. Hope you guys can shed some light on what I'm doing wrong! I'm a pretty experienced front-end developer, but I'm kinda learning the ropes with server-side stuff. Thanks :)

ninjaEdit: I found this question Are these two functions overkill for sanitization? which is definitely helpful in making my sanitize functions better, but it doesn't really help me with why mine aren't working in the first place.

Community
  • 1
  • 1
  • sanitizeMySQL should be `if (get_magic_quotes_gpc()) $var = stripslashes($var);` __then__ `$var=mysql_real_escape_string($var);` only. Your messing with the mysql encoded string, so its no longer escaped correctly – Waygood May 14 '13 at 09:27
  • have you considered using pdo and prepared statements? If you use this engine you don't need to care about escaping when you bind param PDO does it for you. – Robert May 14 '13 at 10:03
  • I started using PDO and prepared statements. Thanks for the suggestion! Still not sure if I need to sanitize more than just using bind params though – user2380943 May 15 '13 at 21:01

2 Answers2

1

why did you put ' around $var ?

in the function get_post you should do return sanitizeMySQL($_POST[$var]);

you should use prepared statement so it escapes character for you

atrepp
  • 2,114
  • 1
  • 13
  • 13
0

mysql_real_escape_string() should be the last thing you do, to make the string ready for database use. Playing around with the escaped string AFTER mysql_real_escape_string could render the string useless for queries

// Sanitize Functions
function sanitizeString($var)
{
    if (get_magic_quotes_gpc()) $var = stripslashes($var);
    $var = htmlentities($var);
    $var = strip_tags($var);
    return $var;
}    

function sanitizeMySQL($var)
{
    $var = sanitizeString($var);
    $var = mysql_real_escape_string($var);
    return $var;
}    

function get_post($var) 
{
    return sanitizeMySQL($_POST[$var]);
}

p.s. get_post() would be better written if you pass the variable rather than the name of the post index.

ie

function get_var($var) 
{
    return sanitizeMySQL($var);
}

get_var($_POST['message']); 
get_var($_POST['message'][0]); // array elements on the form 
get_var($_GET['index']);  // GET parameter
get_var($_FILES['file']['name']);  // original name of uploaded file
Waygood
  • 2,657
  • 2
  • 15
  • 16