I have posted a few threads on this forum relating to a huge loss of data at my website. It was a code flaw, and it was my fault, but a good wakeup call nonetheless.

I have been looking through/posting my code to get tips and opinions on it. One thing that everybody has mentioned I need to look at is preventing SQL Injections.

When somebody fills out a form on my site, for example, depending on what they want to do (i.e: remove post, add post, remove all posts) I assign a code (i.e: code 1, code 2, code 3) and I propagate this in the URL as c=1, 2, 3. Then I handle it using the $_GET command when coding the function

I have been made aware that doing this would make my site EXTREMELY vulnerable to SQL injections. Now my question is this:

$code=mysql_real_escape_string($_GET['code']);
$_SESSION['username']=mysql_real_escape_string($_SESSION['username']);
$post_id=mysql_real_escape_string($post_id);
//etc.
//etc.
//////////////////////////////check if post_id/code is in range for preventing sql injection
if(!($post_id==1 || $post_id==2 || $post_id==3 || $post_id==4 || $post_id==5 || $code==1 || $code==2))
{
	die('Invalid file ID/code. Please contact us if this problem persists.');
}
//////////////////////////////	

//etc.
//etc.
//for example:
$query = "UPDATE posts SET post".$post_id."_name = '$post_name' WHERE username = '".$_SESSION['username']."'";

Would this thwart any sql injection possibilities?

I have gone through all my code and ANYTHING that passes through an SQL command/query I have run through the mysql_real_escape_string() command. Could this hurt me at all? Is this a good or bad idea?

Thanks.

    yes mysql_real_escape_string will prevent sql injections from happening. it escapes characters that could denote the end of a string so the query is still executed as you intended and cannot be modified.

      alright, but can it hurt/ruin anything at all? or it's completely safe to use on any variable?

      if so, why doesn't PHP automatically do it to every variable? Is there a reason to NOT use it?

        it wont hurt anything, if a person inputs malicious data it will escape it so mysql wont take it literally. there is an option in php where you can have it done to a degree, it is the magic_quotes_gpc directive. the only reason many leave it disabled is because if it is on you will always have to call stripslashes on any get, post, or cookie data that could possibly have single or double quotes in it.
        theres no reason not to use it, if you read the php certification guide they explain ALL data to be inserted should be validated and/or run through this or similar functions.

          Hrm, in phpinfo it shows that magic_quotes_gpc is On. If I turn it off it could change a lot on my website, couldn't it? What exactly does it do...I read the php manual about it but got a rough idea of what it does. Could you explain it in lamens terms for me?

          So if magic_quotes_gpc are enabled, is mysql_real_escape_string() useless on my website? What if I propagate an e-mail address through the URL, I won't have to addslashes to it, will I? It's only for text with ' and "

            all magic quotes gpc does is prepend a \ in front of ' " and \0 and incoming data from get, post, and cookie data. these are characters people use to trick mysql into thinking a string is finished, and then its possible for them to add things after it to modify the query.
            if an email address comes from the query string in a url it wont have any slashes but theres nothing dangerous that can be done with an email address. you see on the 3rd mysql_real... example they check for magic quotes gpc and if its enabled they stripslashes and then run the value through mysql_real_escape_string.

              influx, I think that one needs to read up on overall PHP and MySQL security and not just look at one aspect of it.

              There are several things important, like using quotes around values in a query in combination with mysql_escape_string() or mysql_real_escape_string().

              Quote from MySQL general security manual page:

              http://dev.mysql.com/doc/mysql/en/security-guidelines.html wrote:

              A common mistake is to protect only string data values. Remember to check numeric data as well. If an application generates a query such as SELECT FROM table WHERE ID=234 when a user enters the value 234, the user can enter the value 234 OR 1=1 to cause the application to generate the query SELECT FROM table WHERE ID=234 OR 1=1. As a result, the server retrieves every record in the table. This exposes every record and causes excessive server load. The simplest way to protect from this type of attack is to use apostrophes around the numeric constants: SELECT * FROM table WHERE ID='234'. If the user enters extra information, it all becomes part of the string. In numeric context, MySQL automatically converts this string to a number and strips any trailing non-numeric characters from it.

              You ask "why doesn't PHP automatically do it", well it does do an addslashes() when magic quotes is on. Most web hosts have this option on by default. So, you need to know those settings first before doing addslashes/mysql_escape_string manually. See this post of mine: http://www.phpbuilder.com/board/showpost.php?p=10648400&postcount=9

              I don't see where $post_id is first set before here:
              $post_id=mysql_real_escape_string($post_id);

              If you're relying on register_globals being on, then turn it off for security reasons and program accordingly. That means using $GET, $POST, $_COOKIE, etc.

              Remember that all HTML data comes across as strings. So, if you're expecting something numeric you can do it this way:

              $code = isSet($_GET['code']) ? intval($_GET['code']) : 0;  
              
$post_id = isSet($_GET['post_id']) ? intval($_GET['post_id']) : 0;

if ($post_id < 1 || $post_id > 5 || $code < 1 || $code > 2)
{
    die('Invalid file ID/code. Please contact us if this problem persists.'); // or redirect to home
}

              This is just a quick example and more needs to be in place but you get the idea. Note the intval() allows values like "12test" but makes it a number 12. So, remember that the $_GET value will still have "12test" in it. But it would assign zero if the input was this way: "test12".

              I can see from the code snippet you posted that you have column names with "postx_name" (where 'x' is a number) and I think this is a wrong approach. Something like that should be a row value and not a column name. Make sure your database structure is done right to begin with and is normalized to at least the 3rd normal form. See this Introduction to Database Normalization page: http://dev.mysql.com/tech-resources/articles/intro-to-normalization.html

              Look at the security links in my signature too.

              hth.

              🙂

                Question:

                On the site I allow people to make posts, and when they are editing their posts in their member pages, I use a WYSIWYG editor ("RTE: Rich text editor").

                With the webapp came the following function for inputting the formatted text into a database safely:

                function RTESafe($strText)
{
	//returns safe code for preloading in the RTE
	$tmpString = trim($strText);

//convert all types of single quotes
$tmpString = str_replace(chr(145), chr(39), $tmpString);
$tmpString = str_replace(chr(146), chr(39), $tmpString);
$tmpString = str_replace("'", "&#39;", $tmpString);

//convert all types of double quotes
$tmpString = str_replace(chr(147), chr(34), $tmpString);
$tmpString = str_replace(chr(148), chr(34), $tmpString);
//$tmpString = str_replace("\"", "\"", $tmpString);

//replace carriage returns & line feeds
$tmpString = str_replace(chr(10), " ", $tmpString);
$tmpString = str_replace(chr(13), " ", $tmpString);

return $tmpString;
}

                Should I keep this function? Does it do the same thing as mysql_real_escape_string()?

                  I have no idea man. I visited that site already...obviously I read the comments in the function, but I'm wondering if mysql_real_escape_string() covers all the replacements the function does.

                    Don't be afraid to just try stuff out yourself. That's partly how we learn.

                    Try this for an example and see what you get. Try and guess what it might display before running it:

                    $line = '"Hello" ' . chr(147) . 'World' . chr(148);
echo 'Escaped: ', mysql_escape_string($line), '<br>';

                    Display the characters that the code is focusing on to see what they are:

                    for ($i=145; $i < 149; $i++)
    echo "chr($i): [", chr($i), ']<br>';

                    Experiment and have fun!

                    FYI - I would not use the last part of replacing the carriage return and line feed (if you want to keep the data the same as it was inputed).

                      Write a Reply...