0

Hi all im experimenting and started creating a website that is going to implement a forum. I have starting looking at security issues that may effect the website. Area's such as cross site scripting and sql injection attacks.

From research stripping all html tags will prevent the XSS. Will this along with stripping the SQL special characters going to be enough to prevent the sql injection attacks?

<?php
require "dbconn.php";

$mnam = strip_tags($_GET['blogentername']);
$mcom = strip_tags($_GET['blogmessage']);
$approve = 'N';
$dte = gmdate("d-M-Y H:i:s");

$mnam = mysql_real_escape_string($user);
$mcom = mysql_real_escape_string($mcom);

$query = "INSERT INTO blogentry VALUES  ('".$mnam."','".$dte."','".$mcom."','".$approve."','','')";

$results = mysql_query($query)
           or die(mysql_error());

header('Location:messages.php?messagesent=1');

?>

Thanks all for you time Andy

user2141414
  • 101
  • 1
  • 3
  • 10

3 Answers3

0

You should never strip tags. Save them in the database raw, then use htmlspecialchars when outputting them to the browser.

Niet the Dark Absol
  • 320,036
  • 81
  • 464
  • 592
  • I don't think this is very sound advice. It might be pretty easy to forget that the user has control over information being retrieved from the database, whereas it's very easy to realize that a user has control over data while inserting it into the database. Why not do both? – Alex Lynch Apr 19 '13 at 19:27
0

For the code given - yes, it is pretty safe and impenetrable.

However, your idea of defending from SQL injection is quite wrong in general.

There is no "stripping of SQL special characters" in your code. And there is no point in such a stripping at all.
Speaking of SQL strings, they are pretty safe if properly formatted. But for all other SQL literals, the thing you call "stripping" will actually strip nothing.

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
0

As a best-practice, never use string concatenation to build your SQL commands.

Use parameterized queries. They are inherently much safer and can be optimized by the database engine to provide better performance when doing the same type of statement multiple times.

More information with examples here: How can I prevent SQL injection in PHP?

Community
  • 1
  • 1
diamondsea
  • 2,980
  • 1
  • 12
  • 9
  • Unfortunately, we can't avoid concatenation. So, it's better to know the rules – Your Common Sense Apr 19 '13 at 19:33
  • Doing the same type of statement multiple times in a typical web-application smells of bad design – Your Common Sense Apr 19 '13 at 19:34
  • It is often done when doing INSERTs or UPDATEs while looping through a dataset. The SQL statement is prepared one time, and then the values are simply passed to it each time through the loop and quickly executed, because it does not have to re-evaluate the entire SQL statement. See http://stackoverflow.com/questions/1318023/performance-of-parameterized-queries-for-different-dbs and http://stackoverflow.com/questions/1851834/why-is-using-a-parameterized-query-to-insert-data-into-a-table-faster-than-appen for more info. – diamondsea Apr 20 '13 at 03:58
  • This imaginary "evaluation" you are talking about as though it's something big and considerable, in reality takes such a small amount of time that you hardly ever notice it. Especially for such a simple query like insert or update. Especially when compared to HDD write operation. First answer you linked to were using SELECT for tests and second one have no proof at all. And, I suppose, your opinion is based not on your own experience, but on some articles you read? – Your Common Sense Apr 20 '13 at 05:02
  • No, I haven't personally benchmarked it. That's why I passed on the comments which referenced people who did: "I've seen sqlite becoming 3 times faster when you use parameterized queries, oracle can become 10 times faster when you use parameterized queries in some extreme cases with a lot of concurrency." and " In certain cases I’ve seen 5x+ performance improvements when really large amounts of data needed to be retrieved from localhost". Like all optimizations, figure out where your bottlenecks are first. This will probably not be the worst thing in a program, but it's something to check. – diamondsea Apr 20 '13 at 06:06
  • Also, the people who write the databases recommend doing this, so I passed it along as a "best-practices" tip. "Parameterized queries yield better performance by compiling the query only once and executing the compiled plan multiple times..." http://msdn.microsoft.com/en-us/library/ms172984%28v=sql.100%29.aspx – diamondsea Apr 20 '13 at 06:07
  • Judging by nature of prepared statements, I would say that they have nothing to do with "really large amounts of data needed to be retrieved from localhost". And judging by "10 times faster" number I would rather consider measurement error rather than real improvement. – Your Common Sense Apr 20 '13 at 06:09
  • Having cited the recommendations of the developers of the databases themselves, I'm not sure what you are looking for as far as an answer, since you've already dismissed the claims of people who did test it firsthand as measurement error. What is your personal recommendation on this issue and why? – diamondsea Apr 20 '13 at 07:44
  • I find a speed difference (if any) too negligible to be mentioned. I find it misleading and exaggerated. Many people repeat that mantra of " better performance" but never criticize it nor test in a real life environment. As for the native prepared statements - they are just exaggerated yet very limited feature, not a silver bullet by any means. It's easy to answer "use prepared statements" but it's much harder to follow this advise in a real code. – Your Common Sense Apr 20 '13 at 07:59