Possible security flaw php and SQL

Question

So I have this code which basicaly retrieves data from a mysql database:

$categoria = $_GET['categoria'];
if($categoria ==""){}else{
$consulta = @mysql_query("SELECT * FROM productos where categoria='$categoria' ORDER BY     nombre ASC");
while($seleccion = @mysql_fetch_array($consulta)){
$nombre = $seleccion['nombre'];
$referencia = $seleccion['referencia'];
$descripcion = $seleccion['descripcion']; 
$imagen = $seleccion['imagen'];

And well after that I echo all of the variables... I was wondering, might there be any problem regarding security with a code like this? Is there any risk of it being hacked? Thanks!

BTW, you can use if($categoria !=""){ or if($categoria){ instead of if($categoria ==""){}else{. — Petr R.
– Petr R., Commented Jul 13, 2013 at 10:49
you can't ask two questions in one place. when one question is answered if you have another question ask a new question. — DevZer0
– DevZer0, Commented Jul 13, 2013 at 12:24

samayo · Accepted Answer · 2016-07-14 07:19:39Z

2

I was wondering, might there be any problem regarding security with a code like this? Is there any risk of it being hacked?

Yes, it does! Not only are mysql_ functions deprecated and no longer being maintained, partly due to the security risk they allow by the use of SQLInjection, they also lack also object oriented capabilities.

The best secure way of connecting and handling database queries that exists now is, either via the mysqli or PDO interfaces. you should learn either one that suits.

Last but not least, suppressing errors using the @ method, is pretty much telling your script, not to show you errors, when they occure. Remember, errors should be dealt-with not ignored

edited Jul 14, 2016 at 7:19

answered Jul 13, 2013 at 10:42

samayo

16.5k13 gold badges95 silver badges115 bronze badges

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

Comments

Community · Accepted Answer · 2017-05-23 11:58:56Z

1

With your existing setup of mysql extension the best approach to follow is at least passing the strings through mysql_real_escape_string. If you can move to mysqli or pdo based setup that would be ideal.

$consulta = @mysql_query("SELECT * FROM productos where categoria='" . mysql_real_escape_string($categoria) . "' ORDER BY     nombre ASC");

Also a side note from Simon its best you don't prefix the statement with @. This adds a bit of overhead to the execution. You should handle errors with error_reporting and display_errors.

edited May 23, 2017 at 11:58

CommunityBot

11 silver badge

answered Jul 13, 2013 at 10:44

DevZer0

13.5k7 gold badges29 silver badges54 bronze badges

3 Comments

samayo Over a year ago

Don't forget to remove / mention suppressing errors with @ symbol is a bad idea.

Daniel AG Over a year ago

if i move to mysqli, would I have to make any modifications to the code?

DevZer0 Over a year ago

yes you would need some modifications. all the _query functions need to be added the connection parameter. but that just the basic migration to use the mysqli extensions benefits such as prepared statements you will have to overhaul the code. @DanielAntónGarcía

Vivek Sadh · Accepted Answer · 2013-07-13 10:36:43Z

1

This code definitely has serious security flaws. You should use Prepared statement instead. The GET parameter can be modified. Your code is vulnerable to SQL Injection.

answered Jul 13, 2013 at 10:36

Vivek Sadh

4,2683 gold badges35 silver badges55 bronze badges

Collectives™ on Stack Overflow

Possible security flaw php and SQL

3 Answers 3

Comments

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related