0

Is it safe to use this code ?

$check = mysql_query("SELECT id FROM table WHERE nick='asd'");
$count = mysql_num_rows($check);

I just need number of rows. id is AUTO_INCREMENT

OdkoPP
  • 442
  • 6
  • 14
  • 1
    Is this the final statement? I mean, you dont want to put a variable in replace of 'asd' ? Then you are safe! – xyNNN Jan 07 '14 at 15:48
  • do you get your id from user input? – Alireza Fallah Jan 07 '14 at 15:48
  • use mysqli or use PDO mysql_ is deprecated. – Sam Jan 07 '14 at 15:48
  • `num_rows` is irrelevant, that's not where injection occurs. [The Great Escapism (Or: What You Need To Know To Work With Text Within Text)](http://kunststube.net/escapism/) – deceze Jan 07 '14 at 15:49
  • I will use PHP variables instead of asd – OdkoPP Jan 07 '14 at 15:49
  • how do you generate the php variables? do you get it from post or get ? put your complete code in your question – Alireza Fallah Jan 07 '14 at 15:50
  • variable will come from SESSION. I have no more code .... it is just an idea that will come true but i want to know the best way and the easiest way. – OdkoPP Jan 07 '14 at 15:56
  • possible duplicate of [How can I prevent SQL injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – molnarm Jan 07 '14 at 16:11

2 Answers2

4

If 'asd' is a constant and not related to any (user) input, then yes it is safe.

Otherwise you should replace it with bind a variable and use prepared statements or at least escape it properly. (But it is easy to forget escaping, so it is a better practice to try to use bind variables instead.)

Lajos Veres
  • 13,595
  • 7
  • 43
  • 56
  • 1
    The question is: "Is it safe to use this code ?" Or Am I missing something? – Lajos Veres Jan 07 '14 at 15:49
  • 1
    No, you are correct. He gave two correct answers - is `asd` a constant, yes it's safe. If it's not, then no, it's not safe. – Mave Jan 07 '14 at 15:54
  • yes question is as you said ... so escape inpus should be enought ? – OdkoPP Jan 07 '14 at 15:54
  • If you do it properly everywhere then yes, escaping is enough. But generally it is better to use prepared statements. – Lajos Veres Jan 07 '14 at 15:56
  • and if i use only mysql_query("SELECT id FROM table") and then mysql_num_rows it will be safe ? so i mean to count all rows – OdkoPP Jan 07 '14 at 16:09
  • Sql injection is only relevant if you insert user input to your queries. So constant queries are fine. – Lajos Veres Jan 07 '14 at 16:12
1

NO. Absolutely not.

First of all, read up on MySQLi. The i stands for improved. Secondly, use prepared statements. This prevents injection. Read up on that here.

$db = new mysqli("localhost", "DATABASE-NAME", "DATABASE-USER", "DATABASE-PASS");
$check = $db->prepare("SELECT `id` FROM `table` WHERE `nick` = ?");
$check->bind_param('s', $nickVar);
$check->execute();

Don't take the easy way out. Keep doing things safe until it comes naturally. I used to be all about quick hacks, quickly get it to work, quickly write some things down, but in the end, it's best to get used to good practice.

Mave
  • 2,413
  • 3
  • 28
  • 54
  • i wanted to avoid to use PDO here ... i´m able to use it but it is too long I think to use it just to count rows. – OdkoPP Jan 07 '14 at 15:52
  • 1
    Define 'too long'. A few more lines definitely are worth the vulnerabilities you avoid by using PDO. – Mave Jan 07 '14 at 15:53