views:

270

answers:

6

Hi,
So, members of my website can post topics, replies, comments, edit them and so on. I always use htmlspecialchars and addslashes for html inputs to protect my site against XSS and SQL injection attacks. Is it enough or is there something more I miss?
Thanks.

+2  A: 

A better approach to protect against SQL injection is to use the escape function specifically written for each database - for example, for PostGreSQL use pg_escape_string to escape string fields before inserting them in to the database. Or in your case, use mysql_real_escape_string.

Justin Ethier
Even if I use `addslashes` the hacker can using SQL injection hack my site? It's not enough?
hey
@Tom it clearly states in the documentation for addslashes that you should use the database specific escapes: http://www.php.net/manual/en/function.addslashes.php
baloo
I see. But can you give me an example when using a `addslashes` function wouldn't be enough?
hey
A quick google shows: http://shiflett.org/blog/2006/jan/addslashes-versus-mysql-real-escape-string I don't see why you wouldn't use the escape functions that are created for the purpose and recommended by the php documentation
baloo
I used everywhere `addslashes` instead of `mysql_escape_string`, so that's why... But probably I will have to change it.
hey
baloo: the best reason I can think of for the resistance to switching away from addslashes() is if he's already got addslashes in dozens or hundreds of places all over his code base. And there's a lesson there: always use *some* kind of database abstraction layer, even if it's only a handful of hand-written generic wrapper functions. Don't sprinkle direct database-related function calls all over your code.
Frank Farmer
Yeah Frank, you are absolutely right... Good lesson, I will absolutely use it for next projects.
hey
So then sql injection is the only vulnerability that affects web applications? The op didn't ask how to stop sqli...
Rook
Agreed, and you put together a nice list. However, I think it is important to point out that he is using the wrong approach for preventing SQL injection. So his site is even less secure than he expected.
Justin Ethier
@Justin Ethier Your right, addslahses() is not good.
Rook
+2  A: 

You should use mysql_real_escape_string() for SQL, not addslashes. (Assuming you are using MySQL)

baloo
Even this is not a magic pill. sqli is still possible in values not delimited by single quotes. Validate data fits expected patterns and use *bound sql parameters* to guarantee injection-safety and forget about string-escaping functions.
Cheekysoft
Yes this is just one of many things in the overall security. Always design the code in a security perspective, don't wait until later
baloo
+4  A: 

You should use prepared statements (see PDO) to prevent SQL injection. When outputting the content htmlspecialchars() seems sufficient to prevent XSS.

Also take a look at these links for more ways to protect your site:

http://phpsec.org/projects/guide/

http://cwe.mitre.org/top25/#Listing

http://www.owasp.org/index.php/Top_10_2010-Main

TheMagician
This should be upvoted more than the one using mysql_real_escape_string.
Lotus Notes
+1. PDO/mysqli is what you should use. It is much more secure (and cleaner) than having to escape everything in queries.
poke
htmlspecialchars() is only safe in certain areas of the page and even then, only when used correctly. See http://stackoverflow.com/questions/2964424/to-htmlencode-or-not-to-htmlencode-user-input-on-web-form-asp-net-vb/2965444#2965444 and http://stackoverflow.com/questions/110575/do-htmlspecialchars-and-mysql-real-escape-string-keep-my-php-code-safe-from-injec
Cheekysoft
A: 

When inserting data into database, use prepared statements. PDO are better than mysql_real_espace_string.

When displaying data, such as comments, posts, use htmlentities.

ggfan
PDO and ADODB use mysql_real_espace_string() when they are connected to mysql.
Rook
A: 

SQL injection:

  1. No addslashes nor mysql_real_escape_string could help alone. But only when used according some rules. And even then it's not enough. So, that's why prepared statements are way better for newbies - it require no thinking.

  2. Both escaping and prepared statements can help with data only. For the operators/identifiers there are distinct rules. (Not a big deal though - every possible combination must be hardcoded in the script)

XSS:

Do not allow users to use HTML.
To prevent this, both strip_tags() (with no allowed tags) or htmlspecialchars() can be used.
If you want to allow some markup, consider a BB-code use.

CSRF:

Any significant form must contain an unique token, which should be compared to one, saved in the session.

Col. Shrapnel
So then there are only 3 vulnerabilities that affect web applications? I think you should read my post...
Rook
XSS is more complex than this, colonel. Please see http://stackoverflow.com/questions/2964424/to-htmlencode-or-not-to-htmlencode-user-input-on-web-form-asp-net-vb/2965444#2965444
Cheekysoft
+5  A: 

There is a lot that can go wrong with a web application. Other than XSS and SQLi, there is:

1)CSRF - Cross Site Request Forgery
2)LFI/RFI -Local File Include/Remote File Include caused by include(), require()...
3)CRLF injection in mail()
4)Global Variable Namespace Poising commonly caused by register_globals,extract() import_request_variables()
5)Directory Traversal: fopen(), file_get_contents() file_put_conents()
6)Remote Code Execution with eval() or preg_replace() with /e
7)Remote Code Execution with passthru(), exec(), system() and  ``

There is a whole family of vulnerabilities regarding Broken Authentication and Session Management which is apart of the OWASP Top 10 that every web app programmer must read.

A Study In Scarlet is a good black paper that goes over many of these vulnerabilities that I have listed.

However, there are also strange vulnerabilities like this one in Wordpress. The definitive authority on what is a vulnerability is the CWE system which classifies HUNDREDS of vulnerabilities, many of which can affect web applications.

Rook
+1 for a great list and direct answer to the OP's question, although the paper itself is a bit dated (from 2001 and only discusses PHP 4).
Justin Ethier
@Justin Ethier you are correct that paper is a bit old however a lot of the "sink functions" are still problematic. I also really like the part about "language vs programmer". It also has example vulnerable code to back my list, so I still think its relevant.
Rook
Till now the best answer. Probably you are good hacker, lol. Thank you.
hey
@Tom Thanks, I do my best.
Rook