1

I've read up on SQL Injection, XSS and other security issues and am trying to figure out what to use to safeguard the company's site.

We are about to deploy a simple 'User feedback' form with a textarea so users can tell us how to improve the site to enhance their user experience.

When the user pushes 'submit' on the form, we read the textarea comments from the user, and then programmatically create a filename in that user's subfolder and save their comments to a file. Then we add the filename and path to that user's database record.

The team is not worried about security issues here but I am. Their thinking is "we create the filename, it is 0% based on any user input, and since we write this 'UserX comments' filename and path to the database with no direct user influence -- there is no risk."

My concern is NOT the database activity -- because they're right, the user has no role in WHAT we write to their database record since we're just creating our own filename and storing it in their db record.

My concern is the text file!

So I'm petitioning our small team to rewrite the code to use security to read then write the user's comments in the textarea to the text file.

My concern is -- since we plan to actually READ our user's feedback and open these text files to read them later on -- there might be bad stuff in the textarea that (unless we clean it) could hurt us somehow.

I'm insisting we use strip_tags() but I need to sound informed about the manner in which we sanitize the textarea input -- I'm thinking strip_tags() is the way to go here but I'm 100% new to sanitizing user input. I looked at htmlspecialchars() but that just converts certain characters like '&' to & and so forth.

Are there other ways to santize/make safe any text the user types into a textarea before we write it out to a file on our web server?

wantTheBest
  • 1,682
  • 4
  • 43
  • 69
  • `htmlspecialchars` also converts `<` and `>` into `<` and `>` and this is enough to prevent displaying arbitrary html from use input. – kirilloid Mar 10 '12 at 03:23
  • 2
    I think this begs the question: why are you storing these in text files rather than a database? – Andrew Marshall Mar 10 '12 at 03:24
  • AM -- good question -- since this is a 100% new feature, we have no clue how much it will be used so for now we're not adding code and database mods to handle the security issues etc. but if after rollout it turns out to be a not-infrequently used feature of the site then storing it in the db might be better. – wantTheBest Mar 10 '12 at 03:29
  • 1
    'security' on reading user-provided data depends ENTIRELY on how you're planning on reading it. suggesting html sanitization is entirely useless if you're just going to dump the file to a printer, and it's got embedded PCL or EPS that causes your printer to catch fire. – Marc B Mar 10 '12 at 04:38
  • 1
    Col. Shrapnel you changed your handle to 'Your common sense' -- you're a TROLL on this fine forum and I just flagged you to have the site admin see if you need to be prevented from making a fool of yourself in these posts. Seeking attention by being an abrasive ahole is dumb. Ask yourself this question: "did I offer valuable feedback like the other excellent and helpful responses? Or am I an abrasive, attention-seeking ahole TROLL?" Regardless of your introspection and highly-deserved self-recriminatations, I have flagged you to ask the Admin to witness an abrasive troll. – wantTheBest Mar 10 '12 at 04:55
  • Seeking only positive feedback will never do you any good. But it's up to you, if you want not education but only a comfort here :) – Your Common Sense Mar 10 '12 at 05:11
  • Col Shrapnel aka your common sense -- If you have nothing helpful to say - don't say it; if you HAVE something helpful to say and you don't know the old saying "It's not what you say but how you say it" -- then you're right I'll take the excellent help of the 99.99% of the other S.O. aficionados who are smart enough to know you can help folks without saying the crap I've read from you in so many other posts -- NOT helpful dude. I 100% tell you to stay out of my posts. If S.O. had an 'ignore' button you'd be long gone I'm sure. – wantTheBest Mar 10 '12 at 07:36
  • People who voted me 40k+ reputation think otherwise. May be there is something wrong with your questions, not my answers? – Your Common Sense Mar 10 '12 at 11:41

6 Answers6

3

Looks like strip_tags is a good way to go. I'd also suggest writing the file outside of the webroot so that it can't be accessed by a browser. See also: This Other Thread

Community
  • 1
  • 1
TecBrat
  • 3,643
  • 3
  • 28
  • 45
3

If you're not worried about SQL injection, and it seems you're not (either because you know the SQL is sanitized or because you're saving to a text file), then the other problem is the possible XSS attack.

It's easy to ignore those, they don't affect you directly. An XSS attack is an attack that allows one to inject client-side scripts into a webpage. Your database works fine, your server files are not modified, your session files aren't modified either.

This vulnerability is completely client-side. Like I said, it doesn't affect your server. But then someone (i.e.: me) goes on your website, and all of a sudden is redirected to a Warez site while viewing a totally SFW, trusted website. You lose trust from your users. The search engines that crawl your site also mark you as possibly harmful. You lose traffic. You lose revenues. Then again, your server is perfectly fine.

You definitely need to sanitize the user input that is outputted back to the user because of this. Yes, strip_tags is a solution and so is htmlspecialchars or htmlentities.

strip_tags is a little less restrictive however, because it allows you to define some tags you'd like your users to be able to insert in their posts, like bold, links, or italic.

In conclusion, you are absolutely right on insisting on this practice. It doesn't affect you (i.e.: your company's server) directly, but it will affect you at some point if you want a trusted presence on the world wide web.

I know this may be a longer answer than others who should suggest to just strip_tags. They are absolutely right, which is why I upvoted them. Just trying to give you some "corporate" arguments there. :)

Community
  • 1
  • 1
netcoder
  • 66,435
  • 19
  • 125
  • 142
  • +1 there - wow client-side script injections (sounds like XSS a bit). I'm going to read up on it. At some point when this goes live as a couple folks suggested above, the project will evolve like any site does and eventually this 'user feedback' input may become user-viewable and editable after the 'Submit' and may be saved to a database -- client-side script injection, man. "You definitely need to sanitize the user input that is outputted back to the user because of this." Thanks for that clarification. – wantTheBest Mar 10 '12 at 04:04
2

It depends on how you are creating a file, and what are you doing with the text after reading it.

If you are using PHP's native functions to write the file, then you should have no problem with remote code execution.

If all you do after reading it is display to a user via HTML, htmlentities(), which effectively makes HTML tags inside the text powerless while still displaying correctly to the user, should be enough.

If you are using it as a part of some query to a database, you should use that database cleaning routine before concatenating it to your SQL. (ie. mysql_real_escape_string() for MySQL, or pg_escape_string() for PostgreSQL).

You may also want to take a look at some info on the OWASP page.

Edit: I forgot to mention, you should also use ENT_QUOTES with htmlentities to prevent single quote injections.

Nathan
  • 4,017
  • 2
  • 25
  • 20
  • Yes it's basically reading the 'value' of the textarea into a javascript variable then using the PHP file functions to save it to a text disc-based file. Initially it will be a rotating task on the team to read all feedback and make a presentation to all of us and to prioritize what are 'must haves' (things lots of users want/complain about). – wantTheBest Mar 10 '12 at 03:33
  • I should say that users do not see their comments after they push 'submit' -- their comments simply are saved to a disc file and a team member reads them. There's no UI on our site for a user to read or edit their previously-submitted comments. – wantTheBest Mar 10 '12 at 03:39
  • +1 Nathan thanks for the great links -- especially the OWASP site -- man this site security is a big big scope. We have some work to do with our site as it stands right now. – wantTheBest Mar 10 '12 at 03:57
  • 1
    @wantTheBest if actually all you do is save the file to the disk, then your only concern should be on how you are saving it. Using functions like file_put_contents or fwrite presents no risk for security. Also, if the file is being read using, say, a text editor, then you need not even bother about HTML filtering. Although its still a good practice to filter it before saving against the most common issues like XSS, even if you are not going to use it now, maybe someone later will and they may not be aware of these issues. – Nathan Mar 10 '12 at 04:02
  • +1 Nathan, exactly, I think the future means this user feedback will in fact be stored in the db and filtering it now as you say means it will 'grow into its shoes' -- as the site evolves our collective attitude on the team vis-a-vis sanitizing input needs to strenghten and right now I'm leading the push for it and with the help here I'm learning what I need to know -- thanks. – wantTheBest Mar 10 '12 at 05:08
1

simply use mysql_real_escape_string() to get rid of quotes. htmlentities() if you are worried about js files. That should be about as good as it gets there.

Jay D.
  • 672
  • 5
  • 12
0

Sanitizing input only protects you from sql injection by removing certain characters. These characters, however, cannot act in a malicious manner from within a text file. I happen to know quite a bit about malware, and trust me, you are not at risk here.

Edit:

If I somehow missed the point of this post through my rambling, do let me know so I can update my answer.

  • +1 thanks for the inspiration for a sigh of relief. I guess I just like the feeling though of knowing before I create a file that I'm not writing any potentially hazardous stuff onto disk. – wantTheBest Mar 10 '12 at 03:52
  • Not a problem, and you most certainly are not. You would have to save the file as an executable file and STILL, the chances of enough data coming through to execute a malicious script would be slim to none. –  Mar 10 '12 at 04:15
0

I have a solution that follows mainstream of your development team ideology:
Do not use any user authorization on your site, including admins. Thus, no XSS will harm you either.

Your Common Sense
  • 156,878
  • 40
  • 214
  • 345