PHP/mySQL Problem

[Nick]

I'm trying to delete both a file on my server and a record from a mySQL database. The following is all the code. I know the server name, database name, and password are right and I starred them out so nobody goes messing up my server. I get an error telling me that there's a syntax problem with my query but it looks like it should work. Do you have to list all columns when deleting a row? I'm only listing a couple because no two files will ever have the same artist and same title, but that might be a problem.


<?
$db_user_name = "*****";
$db_database_name = "*****";
$db_password = "*****";

$artist = $_GET['a'];
$title = $_GET['t'];
$url = $title . '.' . $_GET['e'];

mysql_connect( 'localhost', $db_user_name, $db_password) or die('<br>Failed connect: ' . mysql_error());

mysql_select_db($db_database_name) or die('<br>Failed database: ' . mysql_error());

mysql_query("DELETE FROM 'art' WHERE 'artist' = '$artist' AND 'title' = '$title' LIMIT 1") or die('<br>Failed query: ' . mysql_error());


mysql_close();

unlink('../images/'.$artist.'/'.$url);

echo 'File removed successfully.';
?>

[Carlos Camacho]

This doesn't answer your question, but have you thought about installing PhpMyAdmin? You can quickly (graphically) delete columns, tables, DBs, fields, etc.

[Nick]

I do have phpMyAdmin installed but I'm making a sort of content system for my team to update the page dynamically use a PHP login and then some HTML forms to upload artwork and have the database handle making the page. I actually finished just the other day. It's a rather nice system.

[MacAnthony]

Remove the quotes from your query around the table and column names. I believe MySQL will treat the column names as string and not column names, not matching up to anything.

"DELETE FROM art WHERE artist = '$artist' AND title = '$title' LIMIT 1"

Also I typically don't use a LIMIT command with a delete. The only thing that will do is delete the first row if it finds more than 1 matching row.

[Tesa]

While MacAnthony answered your question, I would like to add the following:

Always parse and verify posted data from the web, and make sure you escape your strings properly before you use them in SQL queries. You don't want any SQL injections (http://www.securiteam.com/securityreviews/5DP0N1P76E.html) or involuntary form insertion mistakes by your team mates.

See the PHP documentation on mysql_real_escape_string (http://se2.php.net/manual/en/function.mysql-real-escape-string.php).

The unlink command needs to be checked as well.

I'd suggest that you:

a) Verify that $artist, $title and $url contain only legit characters, no "../", quotes or other suspicious content.
b) Verify that the desired file exists, then check if the corresponding entry exists in the database.

If both conditions are met, you can proceed to delete the entry from the database (making sure strings are properly escaped) and delete the file from disk.

Keep a log of all attempts where one of either conditions fail to be met. This helps both for debugging the upload/insert part of the system (e.g. there is a file or corresponding entry missing when there shouldn't be), as well as tracking attempts to exploit your system through the most basic means.

Another good check to add is whether the user attempting to delete the entry is indeed a legitimate logged in and registered user with the correct permissions.

[Nick]

All of that is true and in time I'll implement some of it.

The title and URL don't need to be checked as they are generated by my script that adds records and uploads images so I know those will be right. The artist is chosen from a pull down menu so that won't have any weird effects either.

I do check that users are logged in and have a valid user name. As of now, everyone has open permissions simply because the team login only gains access to adding or removing image on our art page. There's no access to any other part of the server gained from loggin in.

The unlink command is fine and in fact the recommended solution i found on php.net (http://www.php.net).