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
– Iłya Bursov, Commented Jan 27, 2016 at 22:34
So I would use it in this example for terms, keywords, db and each? @Lashane — DThomas
– DThomas, Commented Jan 27, 2016 at 22:37
quick check for your script - enter single quote ' as your search query and enable error logging — Iłya Bursov
– Iłya Bursov, Commented Jan 27, 2016 at 22:37
stop using deprecated mysql_* functions completely! Start using pdo or mysqli! Those have better powers to handel sql injection. — davejal
– davejal, Commented Jan 27, 2016 at 22:38
Please stop using mysql_* functions. These extensions have been removed in PHP 7. Learn about prepared statements for PDO and MySQLi and consider using PDO, it's really pretty easy. — Jay Blanchard
– Jay Blanchard, Commented Jan 27, 2016 at 22:49

spencer7593 · Accepted Answer · 2016-01-27 22:58:43Z

4

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.

answered Jan 27, 2016 at 22:58

spencer7593

109k15 gold badges122 silver badges148 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

DThomas Over a year ago

Thank you @spencer7593! I learned about this project using mysql, but will use mysqli functions on my future projects. I appreciate your help!

spencer7593 Over a year ago

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 [xkcd.com/327/] it's explained here [explainxkcd.com/wiki/index.php/Little_Bobby_Tables]. A fuller cheat sheet for SQL Injection is available from the OWASP project [owasp.org/index.php/SQL_Injection].

spencer7593 Over a year ago

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.

Collectives™ on Stack Overflow

Correct usage of Mysql real escape strings

1 Answer 1

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related