Is there a more efficient way to write this MySQL query?

Question

I'm a newbie to mysql, I managed to scrape this together to get the result I wanted. Can it be coded better? Are there any security risks? Its being output in php.

$qwe = $product->virtuemart_product_id;    
$id = mysql_real_escape_string($qwe);

$result = mysql_query('SELECT * FROM virtuemart_product_medias where virtuemart_product_id = ' . $id . ' LIMIT 1');

$row = mysql_fetch_assoc($result);    
$matched = $row['virtuemart_media_id'];

$result2 = mysql_query('SELECT * FROM virtuemart_medias where virtuemart_media_id = ' . $matched . ' LIMIT 1');

$row2 = mysql_fetch_assoc($result2);    
$matched2 = $row2['file_url_thumb'];

echo $matched2;

score 3 · Answer 1 · edited May 23 '17 at 11:49

I don't know whether or not there is a security hole in the specific code you provided - that depends on what other validation exists elsewhere in your program, and what you consider to be a security hole. But the way you are coding means that there definitely could be security holes. Let's look at your first query:

$id = mysql_real_escape_string($qwe);

$result = mysql_query('SELECT *
    FROM virtuemart_product_medias
    WHERE virtuemart_product_id = ' . $id . ' LIMIT 1');

Imagine if $qwe is the string 0 OR 1=1 --. The mysql_real_escape_string only escapes certain characters such as quotes and backslashes.

mysql_real_escape_string() calls MySQL's library function mysql_real_escape_string, which prepends backslashes to the following characters: \x00, \n, \r, \\, ', " and \x1a.

The string 0 OR 1=1 -- that I mentioned above does not contain any of these characters so it will not be affected at all by mysql_real_escape_string. After you substitute in the value of $id, the resulting SQL query will look something like this:

SELECT *
FROM virtuemart_product_medias
WHERE virtuemart_product_id = 0 OR 1=1 -- LIMIT 1

As you can see, this will return all rows.

Long story short: Use PDO and parameterized queries.

Related

How can I prevent SQL injection in PHP?

score 1 · Answer 2 · answered Aug 25 '12 at 15:36

1

Use one query instead of two, and select only the fields you're using, like so:

SELECT `file_url_thumb` FROM virtuemart_medias where virtuemart_media_id = (SELECT `virtuemart_media_id` FROM virtuemart_product_medias where virtuemart_product_id = ' . $id . ' LIMIT 1) LIMIT 1

answered Aug 25 '12 at 15:36

Smok

246
1
7

You beat me at this. deserving +1 :) – verisimilitude Aug 25 '12 at 15:39
This is a dependent sub query, which will execute once for every row returned and can be extremely nonperformant in MySQL. – Interrobang Aug 25 '12 at 15:41
Not in this particular case (LIMIT 1), and only this case is discused here. – Smok Aug 25 '12 at 15:42

score 1 · Accepted Answer · answered Aug 25 '12 at 15:39

Firstly, never use the mysql_* functions. They are deprecated and relying on them is highly discouraged. Use either MySQLi or PDO
The above query could be rewritten as

SELECT file_url_thumb FROM virtuemart_medias where virtuemart_media_id = (SELECT virtuemart_media_id FROM virtuemart_product_medias where virtuemart_product_id = ' . $id . ' LIMIT 1) LIMIT 1
Never do a SELECT *. Include only those fields in your query which you need in your code.

score 0 · Answer 4 · answered Aug 25 '12 at 15:44

You can always use a join;

SELECT a.virtuemart_media_id, b.file_url_thumb 
FROM virtuemart_product_medias a
LEFT JOIN virtuemart_medias b
  ON a.virtuemart_media_id = b.virtuemart_media_id
WHERE virtuemart_product_id = $id
LIMIT 1

That'll always get you the virtuemart_media_id and, if it exists file_url_thumb.

Your query has a problem also, mysql_real_escape_string only escapes strings, since you're not quoting the $id in the query, it won't be handled as a string and the escaping will not help you. As other replies point out, you should really be using mysqli or PDO.

score 0 · Answer 5 · answered Aug 25 '12 at 15:45

0

How about this:

SELECT a.file_url_thumb 
FROM virtuemart_medias a 
LEFT JOIN virtuemart_product_medias b on a.virtuemart_media_id=b.irtuemart_media_id 
WHERE a.virtuemart_product_id=' . $id . ' LIMIT 1

answered Aug 25 '12 at 15:45

Phani Rahul

840
7
22

Is there a more efficient way to write this MySQL query?

5 Answers5