/    Sign up×
Community /Pin to ProfileBookmark

Good SQL Injection and XSS Prevention code

I am doing a lot of CRUD (Create, Read, Update, Delete) on a site I am building for my company. I want to be sure and prevent both sql injection and XSS. I have been reading some books on the matter and found this bit of code. What do you think, good?

[code=php]<?php
$user = mysql_entities_fix_string($_POST[‘user’]);
$pass = mysql_entities_fix_string($_POST[‘pass’]);

$query = “SELECT * FROM users WHERE user=’$user’ AND pass=’$pass'”;

function mysql_entities_fix_string($string)
{
return htmlentities(mysql_fix_string($string));
}

function mysql_fix_string($string)
{
if (get_magic_quotes_gpc()) $string = stripslashes($string);
return mysql_real_escape_string($string);
}
?>[/code]

I have been looking over it for awhile and understand it for the most part, except, it refers to the variable $string. I don’t see how the variable $string is at all referenced in:

[code=php]$user = mysql_entities_fix_string($_POST[‘user’]);
$pass = mysql_entities_fix_string($_POST[‘pass’]);[/code]

Any help understanding how the variable $string is referenced would be appreciated.

to post a comment
PHP

4 Comments(s)

Copy linkTweet thisAlerts:
@NogDogNov 05.2009 — $string is the [url=http://www.php.net/manual/en/functions.arguments.php]function argument[/url] for each of the two user-defined functions in that code. In each case its scope is local only to the function where it is being used.
Copy linkTweet thisAlerts:
@themonkey40authorNov 05.2009 — NogDog -- Thanks for that explanation. So $string is just the local variable to each of the functions. When I call the function:
[code=php]mysql_entities_fix_string($_POST['user']);[/code]

$string than equals $_POST['user']? I think thats right.

What do you think of the code from a security stand point?
Copy linkTweet thisAlerts:
@NogDogNov 05.2009 — Yes, that's correct.

As far as security, the part that does the mysql_real_escape_string() takes care of any issues with SQL injection. As far as XSS and other similar concerns, simply applying a blanket solution of applying htmlentities() may not always be best. Depending on the particular data element in question, it might be better to validate it against a "white list" of allowed characters and return an error if it's invalid, rather than just blindly changing the data. Or if it's text that may need to be searchable, applying HTML character entities to it before storing it in the database could make things confusing when manipulating that data, or even make it too long to fit in a column that would otherwise hold it. (In this case it might make more sense to apply htmlentities() to the data [i]after[/i] retrieving it from the DB and outputting it to the user.)

So read the link I pointed to above and think about what you need to do specifically with each field to ensure that it is "safe," as opposed to simply applying one blanket solution to all fields. In fact, I'd strongly suggest getting your hand's on that author's book: Essential PHP Security.
Copy linkTweet thisAlerts:
@themonkey40authorNov 05.2009 — That some great advice. I was actually reading about that book last night on my iphone. It is currently an app you can by. Will look into it.
×

Success!

Help @themonkey40 spread the word by sharing this article on Twitter...

Tweet This
Sign in
Forgot password?
Sign in with TwitchSign in with GithubCreate Account
about: ({
version: 0.1.9 BETA 6.16,
whats_new: community page,
up_next: more Davinci•003 tasks,
coming_soon: events calendar,
social: @webDeveloperHQ
});

legal: ({
terms: of use,
privacy: policy
});
changelog: (
version: 0.1.9,
notes: added community page

version: 0.1.8,
notes: added Davinci•003

version: 0.1.7,
notes: upvote answers to bounties

version: 0.1.6,
notes: article editor refresh
)...
recent_tips: (
tipper: @nearjob,
tipped: article
amount: 1000 SATS,

tipper: @meenaratha,
tipped: article
amount: 1000 SATS,

tipper: @meenaratha,
tipped: article
amount: 1000 SATS,
)...