8

If I am running a query on a MySQL database using PHP as in the following:

$query="SELECT * FROM tablename";

What is the best way to secure this from things like SQL Injections? I've heard about some escape methods, but won't it leave slashes in the query?

muttley91
  • 12,278
  • 33
  • 106
  • 160
  • 4
    This particular query is not affected by sql injections, by definition – zerkms Aug 15 '11 at 06:27
  • 4
    Is "switch to [PDO](http://www.php.net/manual/en/book.pdo.php) and use prepared statements instead of pasting strings together" a valid answer? – mu is too short Aug 15 '11 at 06:29
  • SQL injections can occur only when there is a `WHERE` clause Suppose your query is `SELECT * FROM tablename WHERE id='$_POST['id']'` Suppose I enter `' OR '1'='1` in the form field for id. Your sql query will now become `SELECT * FROM tablename WHERE id='' OR '1' = '1'` which will always return all fields. – Pranav Hosangadi Aug 15 '11 at 06:42

7 Answers7

14

The query you have shown in the question doesn't use user supplied values so there is no case of SQL Injection but in a general case:-

Firstly, you must validate all the user input(usernames,emails etc.) before using it in a query. For ex:- If you have allowed only alphanumeric characters in a username, then you must check whether the input is actually alphanumeric or not before proceeding to form a database query and you must also check the size of all the inputs.

After that, in my opinion Prepared Statements is the best choice for preventing SQL injection.

Problem with mysql_real_escape_string():-

As mysql_real_escape_string() escapes characters according to default charset, so it is better than addslashes() function and it properly sanitizes SQL injections arising out of abuse of multibyte character sets, but in another article here, a workaround-scenario is shown that explains that injection can still be done.

Solution:-

So the proper and better way of preventing SQL injection is to use prepared statements. It is a technique in which SQL statements are precompiled before the insertion of the user-input (parameters) and are treated as reusable SQL templates. So, it separates the user input from actual SQL-Code and the SQL-parser never parses the user input.

Apart from security, it also optimizes the SQL query for speed. It helps in cases where you need to run same query multiple times with different user inputs.

You can refer to PHP manual for implementation details.

zixtor
  • 241
  • 1
  • 8
  • 2
    When I asked this question, I was far less experienced in SQL. Now that I've learned a bunch, I know that your answer is the best answer. I'm not sure why it had such a low score, but it's very correct. Plus, the mysql_* extension in PHP is deprecated now. Not only that, but in any other language I've worked with using SQL, prepared statements seem to be the best solution. – muttley91 Oct 08 '13 at 01:37
  • 1
    yes, I was also surprised on that negative vote without any vocal objection.. anyway, thanks! – zixtor Oct 09 '13 at 10:18
  • @zixtor, he did not specify what database he is using. Not everytime you can use prepared statement. This Mssql API http://php.net/manual/en/book.mssql.php does not have any prepared statement. What to do in this moment? – scarface Apr 10 '18 at 11:51
8

You shouldn't be doing a select * and should only get the fields you need.

You need to escape text that can be inputted by the user really or using data that is derived from such.

You need to use the mysql_real_escape_string().

Vincent Savard
  • 34,979
  • 10
  • 68
  • 73
Lee Armstrong
  • 11,420
  • 15
  • 74
  • 122
1

first advice, never select *, only select the fields that are necessary, and if all of them are necessary, select individually, so when the project is continued by other developers, they would know whats going on more quicker. secondly, to secure a query use mysql_real_escape_string(); function and if HTML is being passed use htmlentities(); function

Grigor
  • 4,139
  • 10
  • 41
  • 79
  • 1
    "they would know whats going on more quicker" --- if there are 40 fields then reading all of them cannot be really quick ;-) – zerkms Aug 15 '11 at 06:29
  • Can I ask how I would write this so that it would select the specific columns "column1,column2,column3"? I've tried with simply commas, but it returned errors :S – muttley91 Aug 15 '11 at 06:36
  • SELECT `column1`,`column2`,`column3` FROM `table` WHERE `column_name`='' – Grigor Aug 15 '11 at 13:25
1

SQL Injection can be done, when you make something like this

$query="SELECT * FROM tablename WHERE Name LIKE '" . $_GET["name"] . "'";

Attacker can simply put SQL Injection in get parameter name - eg something like "' OR 1 OR '' = '"

Make sure every get or post parameter is passed thru mysql_real_escape_string or at least addslashes + intval .

$query="SELECT * FROM tablename WHERE Name LIKE '" . mysql_real_escape_string( $_GET["name"] ) . "'";
SergeS
  • 11,533
  • 3
  • 29
  • 35
1

from your query i see that there is not security issue.
but, lets say that you want to involve a GET parameter in your query.

the worng way 

$query="SELECT * FROM tablename WHERE id = ".$_GET['id']

here, you have a chance that some one will change the query.
so what you can do is use mysql_real_escape_string
the right way

 $query="SELECT * FROM tablename WHERE id = '".mysql_real_escape_string($_GET['id'])."'";

this way you are protecting the parameter that has being sent by the user.

BUT you should always verify each parameter coming from the user, and on top of that you secure it by the common way as shown above

fatnjazzy
  • 6,070
  • 12
  • 57
  • 83
0

I have used this code kindly see is it the right code and not injection able now

As far as I came to know is : injection code can be injected if we run the insert query? kindly correct me i am not much educated programmer

$rs=mysql_query("Select * from subcat where CATID='".mysql_real_escape_string($_GET['cat'])."' order by ID ASC");
while($row=mysql_fetch_array($rs))
    {

    echo '<td align="left" style="text-decoration:none;padding-left:1px;    "><a href="detail.php?product='.$row['SUBCAT'].'" style="text-decoration:none;color:#000">'.$row['HEADING'].'</a>';
    echo '<td align="CENTER" style="text-decoration:none;padding-left:1px"><a href="detail.php?product='.$row['SUBCAT'].'" style="text-decoration:none;color:#000;">BUY NOW</a>';
    echo '<td align="CENTER" style="text-decoration:none;padding-left:50px"><a href="detail.php?product='.$row['SUBCAT'].'" style="text-decoration:none;font-weight:bold;color:#000">Rs.'.$row['PRICE'].'</a>';
    echo '<tr><td colspan=5 style="border-bottom:1px #232323 solid;">';
    }
user3254166
  • 1
  • Is this an answer, or a question? Questions don't belong to the answer section. Also, `mysql_` is deprecated. `mysqli_` or PDO should be used instead. – John Dvorak Jan 30 '14 at 17:23
0

It's a very old thread, but for the record to other people diverted by search engines: PDO Prepared Statements would be my recommendation.

$connection = new PDO("mysql:host=" . _DBHOST_ . ";dbname=" . _DB_, _DBUSER_, _DBPASS_);

$statement = $connection->prepare("SELECT * FROM tablename WHERE id = :tid");
$statement->execute([':tid' => $_GET['id']]);

$result = $statement->fetch(); // or if you want multiple: $statement->fetchAll()

The nice thing is, that you can build a couple of wrapper classes and functions to ease your live a lot.

This said, usually nowadays a good framework (like symfony) is doing the job already for you. If you only after the database component, you can also integrate just the ORM Database layer into your project via composer: https://www.doctrine-project.org/

Andreas
  • 1,691
  • 1
  • 15
  • 34