1
\$\begingroup\$

I have a comments page on my site and I was wondering if it is vulnerable to some sort of sql attack or something else. At the moment users can type what they want, they can even write a php script and it will upload to the database. Is this a problem and how can I fix it.

This is the script I use:

$name= $_POST['name'];
$email= $_POST['e_mailaddress'];

if(isset($_POST['comments']) AND $_POST['comments']!=''){
    $comments= $_POST['comments'];
    $comments_sent = 'true';

$dbcn = new connection();

$sql= "INSERT INTO contact (name, email, comments) VALUES (:name, :email, :comments);";

$query = $dbcn->dbconnect()->prepare($sql);

$results = $query->execute(array(
":name"=> $name,
":email"=> $email,
":comments"=> $comments
));
}
\$\endgroup\$

3 Answers 3

Reset to default
3
\$\begingroup\$

With prepared statements, I don't see anything that would immediately make it susceptible to sql injection (this isn't to say there isn't something that is there at a deeper level - I'm not a security auditor nor intimately familiar with php). The "or something else" part remains quite large.

Look at OWASP (Open Web Application Security Project) which goes through a large set of possible attack vectors through the web.

Such a page is susceptible to various scripting injection where someone inserts a script (or flash, or even something like an image from another site) to be executed on another client machine. From the OWASP top 10 - 2013 pdf the XSS attack can be seen and described on page A3 (page 9).

\$\endgroup\$
0
\$\begingroup\$

It depends. Prepared statements "help" preventing sql injection when entering data into the database. But at some point you will want to use your data. Thus make sure you check the entered data for consistency and plausibility. e.g. make sure the email address is really an email address and the name does not contain code which might trick your mail routine to send a completely different mail ...

\$\endgroup\$
7
  • \$\begingroup\$ Its just a comments page that only I can read from. I have been posting php scripts and they save into the database but dont show up in the inbox. Any suggestions on a script I could test, nothing malicious just something that will let me know for sure if the scripts in the database can be run. \$\endgroup\$ Commented Jul 6, 2013 at 8:40
  • \$\begingroup\$ Nevertheless, there are bad persons out there, who might craft an entry which, when displayed on your personal comments page sends your own login data to their own server .... \$\endgroup\$ Commented Jul 6, 2013 at 8:47
  • \$\begingroup\$ Aye ive been trying to write something that will let me know if a script will be run when i open my message page. But so far nothing, it just opens message page and has blank entries for the messages that were scripts. My passwords are hashed so even if someone did manage to pul a password from database it will just be a long asss hash \$\endgroup\$ Commented Jul 6, 2013 at 8:54
  • \$\begingroup\$ Well, your question was if the code above prevents any other attack ... and the answer is no, not really ... escape your input variables, check them for consistency and plausibility ... and even then, there could be possible attacks for which your code is prone to ... \$\endgroup\$ Commented Jul 6, 2013 at 8:57
  • \$\begingroup\$ OK so can you tell me what to look into to "escape input variables" and to check for consistency and plausibility? I use to use the function GetSQLValueString back when i was using mysql_query. Is this what you mean? should i be using it with my new script? \$\endgroup\$ Commented Jul 6, 2013 at 9:20
-2
\$\begingroup\$

It is good that you used a paramterized query. I guess you are using pdo and mysqli, if I am not wrong they make a prepared statement and clean it before submitting it to your database.

To protect against sql injection make 2 classes. One that confirms that a name is a name (could check its type that is a string, and length) and an email (check for length and use the made classes on the internet to check that u really an email). - That would be considered as creating a whitelist.

The second class would sanitize bad characters. Sanitize > < " ' % characters that are used for sql injection. The PDO/MSQLi framework that I think you are using, already does some of the filtering..one of your own could also help- That would be considered as creating a blacklist.

WhiteList is safer, but having both of them is good enough.

Make test on your code, if you dont know sql well and ways to use canocolisation to pass the filters, use a good tool which is called sqlmap.

Last note..SQL injection is possible in prepared statements. May be even a secondary sql injection!

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Your last point might be good to clarify: if a value is injected directly into a SQL string (i.e. that part is not inserted as a parameter) then it could be vulnerable. But if the only user input introduced into the query goes through PDO/mysqli parameters, it is 100% safe as far as SQL injection goes. \$\endgroup\$ Commented Jul 6, 2013 at 12:09
  • 2
    \$\begingroup\$ -1. Please don’t ever sanitize. \$\endgroup\$ Commented Jul 6, 2013 at 20:57

You must log in to answer this question.