I don't have much experience with PHP and mySQL, so I really don't know how to write secure code. What I'm trying to do is track the number of times images have been viewed. I'll add a row in a mySQL table with the image name and then link to the image like this:

www.mywebsite.com/images.php?image=folder/imagename.jpg

Then the PHP script would update the number of hits for that image.

Here's the code, it works but I don't know if it's secure (to SQL injections and such..)

$conn = mysql_connect($dbhost, $dbuser, $dbpass) or die ('Error connecting to mysql');
mysql_select_db($dbname);
 
 
$image = mysql_real_escape_string(htmlentities(trim($_GET["image"])));
 
$query = "UPDATE images SET hits=hits+1 WHERE name=" . "'" .$image."'";
 
if(!mysql_query($query))
{
       echo mysql_errno().":";
       echo mysql_error();
}
 
 
echo('<img src="http://www.mywebsite.com/' . $image . '">');
 
?>

Thanks for your advice

What happens if someone inputs an image name of " x OR 1 " ... Validate the input before building the query.

Well since you are quoting an excaped string in your query, you will be safe with injections. Encoding the image name string using htmlentities is not helping with security issues.

I assume that this script file would serve the actual file contents rather than output an HTML string, so you would need to read the file contents out to the buffer. This opens up a whole new can of worms though.

You should validate the given path and filename carefully so someone cannot exploit your script and view the contents of your php, htaccess, or other files. You would also need to check the file system that the file exists and is readable, and set your content type HTTP headers correctly.

If you plan to automate this and do not want to manually keep adding new rows for every image you add to your file system, then you should first count the rows where this path/filename is stored in the db and do an INSERT if the count is zereo or an UPDATE if greater than zereo.

You should also consider URL encoding the image link:
www.mywebsite.com/images.php?image=folder%2Fimagename.jpg

What happens if someone inputs an image name of " x OR 1 " ... Validate the input before building the query.

Could you elaborate? I'm new at this stuff

I assume that this script file would serve the actual file contents rather than output an HTML string, so you would need to read the file contents out to the buffer. This opens up a whole new can of worms though.

Well the code I posted just "echo's" out the line of HTML which has the img tag, so I don't think I'm doing the buffering thing you described?

Well since you are quoting an excaped string in your query, you will be safe with injections.

Is there anything else I need to worry about?

Thanks!

Originally Posted by Nathand

Is there anything else I need to worry about?

Thanks!

No, using what you have is safe enough.

Re x OR 1 Change your query to do a select count(*) instead of the update - see how many rows are returned. How it works is that the db sees "file = x" and "1" and the latter is always true so we match every row ... Also, note especially the reply re other files - you always give a link to the file requested, even if not on the database, so if I can view any file whose name I can guess on your site - and .htaccess/.htpasswd are always fun to catch and crack. That's why I said validate input first and don't do any processing if you don't like the input.

Originally Posted by PaulW

Re x OR 1 Change your query to do a select count(*) instead of the update - see how many rows are returned. How it works is that the db sees "file = x" and "1" and the latter is always true so we match every row ... Also, note especially the reply re other files - you always give a link to the file requested, even if not on the database, so if I can view any file whose name I can guess on your site - and .htaccess/.htpasswd are always fun to catch and crack. That's why I said validate input first and don't do any processing if you don't like the input.

Combining the use of mysql_real_escape_string in a quoted value will prevent any injections.