How can I restore the security of an old site with a sql injection vulnerability which was probably exploited? Edit: Sorry, the question was marked as unclear. In a nutshell, there are two questions:


Does the code below indeed create an sql injection vector? (It seems clear that the answer is yes, even though I was unable to craft a successful attack on my current server architecture.)
Assuming someone successfully exploited that sql injection vector on the old server, what would I have to do to make sure that there are no leftover attack vectors? Would it be sufficient, for example, if I took the following actions:


Move to a new server
Build a new Drupal codebase from scratch
Remove the insecure code from the theme
Block or delete all users except those that are recognized
Force password resets for the recognized users
Check the menu_router table for bad items
Check all node and comment titles, bodies and fields for  tags



...Or is the situation even more hopeless than that, and the site should simply be abandoned or rebuilt from scratch? For various reasons I don't think it is possible to restore from a backup, mostly because the attempted exploits go back 6 years.

=== Original post === 

I'm looking at an old D6 site that was done in a very non-standard way. What concerns me most are some .tpl.php files from the theme that use $_POST parameters to insert data into some tables, e.g.:

&lt;?php 
  $uid = $user-&gt;uid;
  $name = $user-&gt;name;
  $title=$_POST['title'];
  $comment=$_POST['comment'];    
  $nid=$_POST['nid'];
  $cid=db_result(db_query("select  max(cid) from  comments"));
  $cid2=$cid+1;
  $timestamp=mktime();
  db_query("insert  into  comments  values($cid2, 0, $nid,$uid,'$title' , '$comment', '127.0.0.1', $timestamp, 0, 3,'01/', '$name', null, null)");
 ?&gt;


This seems like very straightforward sql injection vector, but I'm not really an expert at exploiting such things, and my relatively feeble attacks have been caught by the new server. However, there are 1000 entries in the database where the titles are pretty clear attempts to execute injection attacks, and worse yet are the 1000 more with empty titles which might represent attacks that were successful.

So how bad is this? Here is my scale:


37GB of spam! Ha ha ha! No problem!
Removing the insecure code and moving to a new server should be safe.
The site will not really be safe until you switch to a new server, remove all the insecure code, and run some automated tests on the database and/or file structure to check for residual attack vectors.
The site will not really be safe until you switch to a new server, remove all the insecure code, and then go through each file and each database row and manually check to make sure that it isn't an attack vector — in other words, it's probably easier to give up and start over, no matter the size of the site.
Theoretically, that SQL query could be used to do anything on that database server, including changing the password for a Drupal  account not used so often, or creating a new account in the users table.

Suppose that $_POST['title'] contains the following.

"ddddd', 'comment', 127.0.0.0.1, 1469987843, 0, 3, '01/', 'Tintin', NULL NULL); DELETE FROM {users} u WHERE u.uid &gt; 0; -- "


This would change ""INSERT  INTO {comments}  VALUES($cid2, 0, $nid, $uid, '$title', '$comment', '127.0.0.1', $timestamp, 0, 3,'01/', '$name', NULL, NULL)" into the following.

"INSERT INTO {comments} VALUES($cid2, 0, $nid, $uid, 'ddddd', 'comment', 127.0.0.0.1, 1469987843, 0, 3, '01/', 'Tintin', NULL NULL); DELETE FROM {users} u WHERE u.uid &gt; 0; -- ', '$comment', '127.0.0.1', $timestamp, 0, 3,'01/', '$name', NULL, NULL)"`


I added a query to delete the users table, but I could have added any query, including the one to alter the password of an existing account, or creating a new user.  

How that type of attack is effective depends only from how much the attackers know about your site: which software you are running, which version, etc. Once they have access as administrator, they could do enable the PHP input format, and execute arbitrary code. I would say that spam is the last of your problems.

As side note, as Clive said, template files should never contain SQL queries; eventually, those could be in preprocess functions, but in most of the cases, they should be in a module.
Also, in Drupal SQL query, you should wrap the database table names in {}. This allows Drupal to use prefixes in the table names, and even use tables from different databases.