views:

337

answers:

2

I have a website where a user can upload images for a real estate property.
The table structure:

image_id
property_id
userid
filename
thumbfilename
display_order
timestamp

The scenario: When a user uploads multiple pictures, he / she SHOULD be able to set the primary photo from their uploaded images for the specified property.

The code:

$sql = 'UPDATE property_images SET display_order = display_order + 1 WHERE property_id = "' . $this->_request->getParam('propertyid') . '"';
$images->getAdapter()->fetchAll($sql);
$images->update(array("display_order" => 1), 'image_id = "' . $this->_request->getParam('imageid') . '"');

The issue: I receive a "general error" when calling $images->getAdapter()->fetchAll(); The SQL is however executed successfully but Zend_DB_Table throws an exception and will not proceed to the next command. Any ideas / suggestions would be appreciated.

A: 

1) First, recognize that you need to fix your code so that you're escaping the user input. You are currently very vulnerable to SQL Injection.

2) Why are you passing an UPDATE query to fetchAll()?

3) Look at Zend_Db_Expr

timdev
I am not vulnerable to SQL injection as this is done using values grabbed from the script itself before the user gets the information. They do not get to pass anything other than the property id and image id. I do a check to verify that the loged in user is the owner of the property and image as well. Thanks anywa though.
Dennis Day
Fair enough. I'd still escape things explicitly, in case that external logic ever changes. Code has a tendency to get reused, and you're relying on authorization code to fail in order to ensure that bad params never make it to the database.
timdev
A: 

Nevermind,

The solution:

$sql = 'UPDATE property_images SET display_order = display_order + 1 WHERE property_id = "1004" AND display_order < 3; $images->getAdapter()->query($sql); $images->update(array("display_order" => 1), 'image_id = "2003"');

This will grab the third image and set it to 1 after setting the images that had display order of 1 and 2 to 2 and three respectively.

Dennis Day