-2

I am just reviewing a script I want to use for my website. The author filters all user entered data ($_POST and $_GET) with this function:

function XSSCheck($value) {
    return preg_replace(
        array('/&(?!amp;|quot;|nbsp;|gt;|lt;|laquo;|raquo;|copy;|reg;|#[0-9]{1,5};|#x[0-9A-F]{1,4};)/', '/#(?![0-9]{1,5};|x[0-9A-F]{1,4};)/',       '|<|',  '|>|',  '|"|',      "|'|"   ),
        array('&amp;', '&#35;', '&lt;', '&gt;', '&#34;', '&#39;'),
        stripslashes($value)
    );
}

If I filter a string with that, would it possible to inject into this SQL query for example?

SELECT * FROM table WHERE ID = '{$_REQUEST['id']}'

I tried it already but did not find out any way to do so.

  • 2
    Did you run the function against possible inputs to see what it does? Just by eyeballing the code I can see that it doesn't filter asterisks and curly braces. Also, the function name indicates it is filtering for XSS and not for SQLi. See the SQLi Cheat sheet for preventing that. – Neil Smithline Sep 19 '15 at 17:13
  • 1
    @NeilSmithline: I do not know why it is named XSSCheck. It also filters for SQL. The question is, provides this function an injection point which mysql_real_escape_string does not? – Richard Sep 19 '15 at 17:16
  • 1
    It is named XSSCheck because it is for preventing XSS attacks (en.wikipedia.org/wiki/Cross-site_scripting). XSS is an attack on the browser, not the database. Use mysql_real_escape_string for preventing SQL injection, use XSSCheck for preventing XSS. – Neil Smithline Sep 19 '15 at 17:24
  • 1
    mysql_real_escape_string can be bypassed, and wouldn't be effective when the injection is numerical. Bind variables should be used instead with whitelisting. – user79537 Sep 19 '15 at 18:06
  • Same question on Stack Overflow – Gumbo Sep 20 '15 at 6:56
6

Blacklists are a bad idea for protecting against vulnerabilities like XSS and SQLi. The reason is that there are so many encoding techniques, browser behaviors, database server inconsistencies, and web server bugs to account for that you'll never be able to build a blacklist of sufficient capability.

For this function, it will not be sufficient for XSS or SQL injection. For example, what about SQL injections outside of string literals? For both classes of vulnerabilities, there are recognized appropriate fixes.

XSS

  • Whitelist where appropriate: A number is a number, a string consists of A-Za-z0-9, etc.
  • Where whitelisting can't be done, such as free-form search strings or comments, output encode.

SQL injection

  • Whitelist where appropriate: A number is a number, a string consists of A-Za-z0-9, the user can only select from tables you specify, etc.
  • Use parameterized queries: PHP supports this. Even better, use an ORM so it takes care of it for you.
  • 1
    What is a purely numerical SQL injection? – Gumbo Sep 20 '15 at 6:59
  • Sure. I admit that was poorly worded. What I'm saying here is not breaking strings. mysql_real_escape_string is meant for strings. It escapes ', ", and a few other characters, but it'll not help with math 10-1 or hex-encoded injections (securitystreetknowledge.com/?p=193) – user79537 Sep 20 '15 at 22:33
1

You can not use that function to prevent SQL injections as @NeilSmithline commented you. The author of that function was right in calling it XSSCheck() because it is what you can use it for.

I feel you are just confused. SQL injection is a server-side vulnerability wheras XSS is a client-side vulnerability. The techniques used to prevent these two attacks are also different. I suggest you to put this big difference in mind and read further about these two notions.

P.S.

Note that the function is missing few special characters such as backslashes.

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.