1

Possible Duplicate:
The ultimate clean/secure function

After reading up on PHP security I have the feeling that anything I code is always insecure. So to combat the security issues of user input I have created a function that allows me to escape and strip user input for any usage situation.

I would just like to know if this is in fact secure and if I could make it more secure. Also what kind of attacks would this prevent? From what I can tell XSS by using _GET, HTML input and MYSQL injection would have been prevented?

function _INPUT($name,$tag,$sql,$url)
{
if ($_SERVER['REQUEST_METHOD'] == 'GET')
    $filter = ($_GET[$name]);//Assign GET to filter variable

    if ($tag == true)//Remove all HTML, PHP and JAVASCRIPT tags
    {
        $filter = strip_tags($filter);
    }
    if ($sql == true)//If MYSQL escaping is enabled
    {
        $filter = mysql_real_escape_string($filter);
    }
    if ($url == true)//If URL encoding is enabled
    {
        $filter = urlencode($filter);
    }
    return $filter;     

}

$output = _INPUT('name',true,true,true);

I will be using prepared statements for MYSQL too, although I need to read up on them more to fully understand how it prevents injection.

Thank you for your time.

Community
  • 1
  • 1
joshkrz
  • 499
  • 3
  • 7
  • 25
  • 1
    If your concerned about security you should not be using the mysql_* functions in php you should you the mysqli_* functions. – Dave_Peachy Jun 12 '12 at 19:57
  • 1
    Also if you want to stop SQL injection 100% have a look at prepared statements. http://php.net/manual/en/mysqli.prepare.php – Dave_Peachy Jun 12 '12 at 19:58
  • 3
    Like Frank_Hemsowrth has said, get rid of all mysql_* functions completely. Treat them as deprecated functions as outlined here: http://news.php.net/php.internals/53799 Definitely look at prepared statements.... And It might be worth your time looking into PDO as your connectivity "agent".... Will make your life a lot easier in the long run... – Justin Jun 12 '12 at 20:02
  • There is no intrinsic security advantage in using mysqli or PDO. They are just more convenient. More convenient to keep things secure. It's a nice side effect, not a feature. – mario Jun 12 '12 at 20:17
  • strip_tags() is only useful against xss attacks when you are injecting into an open html context. To prevent xss in other areas of the html document you need to encode very differently. Please read https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet – Cheekysoft Jun 13 '12 at 08:47

3 Answers3

6

Once again, there is no universal escape function that just magically makes things "secure".

See this: https://stackoverflow.com/a/7810880/362536

Different escape methods are used for different things. You can't just run a bunch of data through a bunch of functions that are supposed to be used in specific contexts. You are creating garbage data, and are no more secure than you were with the raw user data in the first place.

Community
  • 1
  • 1
Brad
  • 159,648
  • 54
  • 349
  • 530
  • I understand this, which is why the function allows me to choose the escaping method. All three filters would probably not be used at the same time. – joshkrz Jun 12 '12 at 20:05
  • 1
    Then why have a function in the first place? How would you specify the order? The whole concept is not a smart way to do it. I'd recommend abandoning the idea and doing this the right way. – Brad Jun 12 '12 at 20:08
  • I find this very confusing, when you say the right way do you mean by directly adding the correct escape method directly to the variable / input? – joshkrz Jun 12 '12 at 20:14
  • 1
    What I'm saying is, you need to understand what all of these functions do and when to use them. In the link I posted to another answer, I outlined this. For databases, use prepared queries (no escaping necessary, since the data isn't simply concatenated into the query). For HTML, `htmlspecialchars()` takes care of using the correct HTML entities, giving you valid HTML and the bonus that someone cannot interject their own script tags and what not into your site. There are others, but I'm running out of room, and the link I posted already has this info... – Brad Jun 12 '12 at 20:16
2

No,

For SQL Injection prevention, you really want to be using prepared statements. This is a safer way to do this, instead of escaping quotes. You also want to use htmlspecialchars() for escaping HTML tags, instead of just stripping them away, but that's up to you.

Oleksi
  • 12,947
  • 4
  • 56
  • 80
1

This is kind of an eternal question, and the answers vary across wanted usage: for prepared queries, I believe it’s 100 % safe to use its own variables system and let it handle the input. For HTML output, stripping tags may not always be what you want; moreover, it’s kind of safer to do a whitelist of what to allow in input than blacklist, because you know, hackers have fantasy. For URL output, your solution should be fine, but be aware that some other platforms may do a little different URL-encoding (see the difference between a string URL-encoded by Java standard libraries and iOS/Mac libraries, i.e.).

themarketka
  • 672
  • 6
  • 14
  • Thanks for the advice. I forgot to mention that I was also going to limit input and validate it according to the format. For example email or telephone number. – joshkrz Jun 12 '12 at 20:07