Correct usage of Mysql real escape strings

Question

I am new to SQL injection and would like to know if I am using the mysql_real_escape_string properly? Should I make strings for the database and password to make this secure? Any advice would be great and highly appreciated!

    <html>
    <body>

<img src="searchlogo.png" style="PADDING-TOP: 50px">
    <form action='./search.php' method='get' >
<label>Search Our Archives</label>
<input type="text" name="k" size="50" value='<?php echo $_GET['k'];?>'/ >
<button type="submit">Search</button>

   </form>
   <hr/>
    <?php
    $k = $_GET['k'];
    $terms = explode(" ", $k);
    $query ="SELECT * FROM search WHERE ";

    foreach ($terms as $each){
        $i++;

        if ($i == 1)
            $query .= "keywords LIKE'%$each%'";
        else
            $query .= "OR keywords LIKE'%$each%'";
    }

   $db = mysql_connect("**host***","***database***","***password***",
        mysql_real_escape_string($user),
        mysql_real_escape_string($password));
    if (!$db) {
    die("Database connection failed: " . mysql_error());
   }


     $db_select = mysql_select_db("***database***",$db);
     if (!$db_select) {
    die("Database selection also failed: " . mysql_error());
    }


   $result = mysql_query("SELECT * FROM search", $db);
   if (!$result) {
   die("Database query failed: " . mysql_error());
   }


   while ($row = mysql_fetch_assoc($result)){
            $id = $row['id'];
            $title = $row['title'];
            $description = $row['description'];
            $keywords = $row['keywords'];
            $link = $row['link'];

            echo "<h2><a href='$link'>$title</a></h2>$description<br /><br      />";
        }


    mysql_close($db);


    $query = mysql_query($query);
    $numrows = mysql_num_rows($query);
    if ($numrows >0){


    }

    else    
        echo "No results found for \"<b>$k</b>\"";
     ?>

     </body>
    </html>

`mysql_real_escape_string` should be used for all text values, which goes to sql queries, don't use it for username/password - as they are not part of sql query — Iłya Bursov, Jan 27 '16 at 22:34
So I would use it in this example for terms, keywords, db and each? @Lashane — DThomas, Jan 27 '16 at 22:37
quick check for your script - enter single quote `'` as your search query and enable error logging — Iłya Bursov, Jan 27 '16 at 22:37
stop using deprecated `mysql_*` functions completely! Start using `pdo` or `mysqli`! Those have better **powers** to handel sql injection. — davejal, Jan 27 '16 at 22:38
in your example you should use it only for `$each` variable, but operator `like` has 2 additional special symbols (`%` and `_`) which are not escaped by mysql_real_escape_string — Iłya Bursov, Jan 27 '16 at 22:38
Please [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php). [These extensions](http://php.net/manual/en/migration70.removed-exts-sapis.php) have been removed in PHP 7. Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [PDO](http://php.net/manual/en/pdo.prepared-statements.php) and [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and consider using PDO, [it's really pretty easy](http://jayblanchard.net/demystifying_php_pdo.html). — Jay Blanchard, Jan 27 '16 at 22:49

score 4 · Accepted Answer · answered Jan 27 '16 at 22:58

No. You've missed escaping the contents of $each before that's included in the SQL text, here:

    $query .= "keywords LIKE'%$each%'";

Our preference would be to avoid using the deprecated mysql_ functions, and use PDO or mysqli interfaces. And we'd prefer to use prepared statements with bind placeholders. They really aren't that hard.

If I was going to "escape" potentially unsafe values included in the SQL text, I'd first break up that assignment, to look like this.

  $query .= "keywords LIKE '%" . $each . "%'";

Now, we can "wrap" that variable reference in a function call, something like this:

  $query .= "keywords LIKE '%" . mysql_real_escape_string($each) . "%'";

You'd need to repeat that pattern wherever you are including potentially unsafe values as part of the SQL text.

And it's not necessary to wrap $username and $password in the connection arguments. That's not a SQL statement. What needs to be "escaped" are values that are included in the text of a SQL statement.

Again, our preference would be to use either PDO or mysqli, and use a prepared statement with bind placeholders. For example:

  $query .= " keywords LIKE CONCAT('%', ? , '%')";

Also, the values you are including in the HTML could include some potentially dangerous content. It's a good practice to run that through proper escaping. In some cases http://php.net/manual/en/function.htmlspecialchars.php is sufficient.

As an aside, when dynamically appending to a string containing SQL text, my preference is to include the required separator space at the beginning of what I'm adding, rather than having to remember to include an extra space at the end of the string I'm appending to. That's just a personal preference.

Thank you @spencer7593! I learned about this project using mysql, but will use mysqli functions on my future projects. I appreciate your help! — DThomas, Jan 27 '16 at 23:06
The other literal parts of your SQL text, for example `FROM my_table WHERE mycol` do not need to be escaped, because those are *literal* values. It's the variables that you include... `$each` that can include "dangerous" characters, e,g **`$each = "' OR 1=1 --`** If you don't understand the classic [https://xkcd.com/327/] it's explained here [https://www.explainxkcd.com/wiki/index.php/Little_Bobby_Tables]. A fuller cheat sheet for SQL Injection is available from the OWASP project [https://www.owasp.org/index.php/SQL_Injection]. — spencer7593, Jan 27 '16 at 23:19
And PDO and mysqli interfaces are not a panacea to SQL Injection; it's entirely possible to write code that is vulnerable using those interfaces. But those interfaces do support *prepared statements* with *bind placeholders*. And using that pattern, we don't need to muck with calling real_escape_string functions because we're not putting variables into the text of the SQL. — spencer7593, Jan 27 '16 at 23:24

Correct usage of Mysql real escape strings

1 Answers1