-2

I've made a function to deal with CSS and XSS injecting but its still getting through.

Someone said the following to me, but I'm not sure what it means:

On your sanitize_input function, do a strip_tags to strip all html tags that may have been added through the form. Read php.net on strip_tags.

Here's my code:

private function sanitizeInput() {
    foreach($_POST as &$post) {
        $post = $this -> db -> real_escape_string($post);
    }
}
halfer
  • 19,824
  • 17
  • 99
  • 186
  • 2
    you are **NOT** doing any sanitizing there at all. you're merely doing some SQL-context escaping, which does NOT do anything to fundamentally "fix" malicious data. Such escaping is also utterly pointless if you're not going to be using that data in an SQL query. – Marc B Jun 04 '13 at 19:14
  • 1
    [The Great Escapism (Or: What You Need To Know To Work With Text Within Text)](http://kunststube.net/escapism/) – deceze Jun 04 '13 at 19:14
  • What functionality are you using to access your database? You should have the strip_tags function too: `$post = strip_tags($this->db->real_escape_string($post));` if you want to remove tags. – Phil Cross Jun 04 '13 at 19:14
  • That looks like it is protecting against SQL injection, not cross-site scripting. The easiest way to do the former is via parameterisation, which is available via PDO and MySQLi - if you are still using `mysql_real_escape_string()` then you need to upgrade anyway, since that library is no longer in use. – halfer Jun 04 '13 at 19:15
  • To remove HTML tags, you should do that when you render, if there should be no HTML at all (the usual case), or when you store to the database if you wish to accept a limited amount of safe HTML. – halfer Jun 04 '13 at 19:16

3 Answers3

1

You're doing the work in the wrong places.

To prevent SQL injection, use prepared/parameterized queries with PDO. This fundamentally separates the data from the command, making it immune to general SQL injection problems.

Your problem with XSS is no doubt because you are using arbitrary data in an HTML context without any escaping. On output of your data, use htmlspecialchars(). This will encode all special characters into their proper entities.

Brad
  • 159,648
  • 54
  • 349
  • 530
  • u mean like $html value="'.htmlentities(htmlspecialchars(stripcslashes($_POST['userName']))).'" /> ? ?? – LearningPhPbasics Jun 04 '13 at 19:24
  • He means use PDO it's a driver for PHP to access mysql. There are good examples on how to use in the link he provided. – Ace Jun 04 '13 at 19:30
  • @LearningPhPbasics, NO! See my answer here: http://stackoverflow.com/a/7810880/362536 You can't just throw a bunch of escaping functions together and expect a useful result. Only escape when you need it, and in the right location. Don't encode your HTML early. Never use `stripslashes()`. Don't use `htmlentities()` and `htmlspecialchars()` at the same time! You're making a huge mess out of your data. – Brad Jun 04 '13 at 19:38
0

The code you have makes your input safe to insert into a database. However, what is safe for a database may not be safe for HTML.

Typically, you shoud use that function to insert the data, and when you retrieve it later you run it through something like htmlspecialchars before outputting it. However, do NOT do this before saving the data, only do it at the final output stage.

Niet the Dark Absol
  • 320,036
  • 81
  • 464
  • 592
0

This depends... do you want to be able to save tags, but not have them rendered in the browser? If so, use htmlentities. If not, use strip_tags.

Perhaps:

private function sanitizeInput() {
  foreach($_POST as $key => $val) {
    $_POST[$key] = $this->db->real_escape_string(htmlentities($val, ENT_QUOTES));
  }
}

It really depends on what you're trying to accomplish.

The example above would convert: A 'quote' is <b>bold</b> to A &#039;quote&#039; is &lt;b&gt;bold&lt;/b&gt;

This solution, however, isn't ideal. You shouldn't convert data before saving into a database. Instead, you'd want to deal with the data after querying it FROM the database.

Rob W
  • 9,134
  • 1
  • 30
  • 50