CODEWITHSUNDEEP
phpsql
Daniel AG
Daniel AG

Reputation: 183

Possible security flaw php and SQL

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!

Upvotes: 0

Views: 124

Answers (3)

samayo
samayo

Reputation: 16495

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

Upvotes: 2

DevZer0
DevZer0

Reputation: 13535

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.

Upvotes: 1

Vivek Sadh
Vivek Sadh

Reputation: 4268

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.

Upvotes: 1

Related Questions