0

I intend to use this for a block of text that will be entered by my users. Of course I'd like to avoid malicious misuse of the site.

From what I have read about MYSQL injections and XSS, I only allow simple text, no HTML tags, no links, nothing special just plain text and some literal links.

Would this suffice?

mysql_real_escape_string(strip_tags($_POST['datablock']));
Community
  • 1
  • 1
Ted
  • 3,805
  • 14
  • 56
  • 98
  • 1
    that absolutely looks safe to me. you're using the appropriate php function to prevent sql injection, and you're stripping all html tags from the input. from there all that's left is text. the only other thing i could suggest would be to run it through htmlentities() to prevent people from adding HTML-parable characters. – Joshua Burns Feb 06 '13 at 23:24
  • 2
    It really depends. Is it safe for your purposes? There are a number of very complex XSS attacks the `strip_tags()` will not help with. Similarly, there are methods of SQL injection that `mysql_real_escape_string()` will not prevent. If you are writing a recipe site with no real personally identifiable data and minimal exposure to XSS risk, this is probably OK. If you are transferring/displaying financial information, it is probably not. How deep down the rabbit hole you go with regards to input sanitation really depends on your use case. – Mike Brant Feb 06 '13 at 23:28
  • 2
    To drive home Mike's point regarding XSS check this link: https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet – ficuscr Feb 06 '13 at 23:30
  • Well, I have to say my position is completely facing the unknown. So I appreciate all comments and opinions here. I would like to ask you @MikeBrant how can the an XSS occur without any possible tag to initiate Javascript? is there other way to do that? – Ted Feb 06 '13 at 23:41
  • @Ted take a look at the link provided by ficuscr above. That give some good examples. To summarize, there are a number of character encoding vectors that could come into play that `strip_tags()` does not handle. – Mike Brant Feb 06 '13 at 23:58
  • It all depends on what you do with the values. So, how exactly do you use these values? – Gumbo Feb 07 '13 at 21:27

2 Answers2

4

Binding your SQL parameters rather than building the SQL string manually is also a good practice (see xkcd on Little Bobby Tables).

Mark Leighton Fisher
  • 5,609
  • 2
  • 18
  • 29
4

Short answer: NO.

The long answer is more complicated.

You must be sure to protect your SQL queries against injection bugs. The best way to do this is to never directly inject user data into your queries, but instead use the SQL placeholders of a database driver to do the insertion for you. This is the safest method by far. Your queries will end up looking like this:

INSERT INTO table_name (user_id, comment) VALUES (:user_id, :comment)

Data is then bound to the various placeholders in a way that the driver can encode it correctly. Being disciplined about this avoids nearly all SQL injection problems.

You must also protect your HTML from XSS attacks by not allowing users to insert arbitrary HTML with scripting into your pages. You should always render user input as the HTML entitized equivalent unless you're absolutely sure that this user content is free of <script> type tags or JavaScript snuck into various elements. Note that this is very hard to do correctly since JavaScript is not confined to script tags at all. It can appear as attributes on an HTML element or even in CSS style definitions.

Further, you should never use mysql_query or any related functions in a new application. These methods are dangerous by default and will cause you severe harm if you miss even one variable. Automated SQL injection tools have a terrifying list of features, and all that these tools require is one unescaped variable.

Thankfully mysql_query is deprecated, it produces warnings in PHP 5.5.0, and will be removed entirely in future versions of PHP.

At the very least you should be using PDO for your database access. It supports named placeholders and makes it very easy to audit your database interface code to be sure it's safe. Mistakes will stand out. As a safety measure, it might be best to define your query strings with single quotes like 'INSERT INTO ...' so that if you make the mistake of putting in a variable it won't be interpolated by accident but will end up yielding a harmless SQL error.

Ideally you should be using a PHP framework to build your applications. Most of these have standardized methods for safe database access, HTML escaping and XSS protection. It is not something you can do on your own unless you want to spend a year writing a framework of your own.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • Why would `mysql_query` be “dangerous by default”? – Gumbo Feb 07 '13 at 21:31
  • `mysql_query` does zero escaping and has no support for placeholders. As such, you are required to use string interpolation to construct your queries. If there's even *one* instance of an unescaped value in your application, it can be cracked wide open. This is an unacceptable level of risk. If you use SQL placeholders and avoid string interpolation entirely, you're safe by default and have to go out of your way to cause SQL injection bugs. – tadman Feb 07 '13 at 23:15