Securely retaining user input in a form

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
Cut
Forum Commoner
Posts: 39
Joined: Sat Aug 23, 2008 8:01 pm

Securely retaining user input in a form

Post by Cut »

Code: Select all

 
<?php
$text = str_replace('</textarea>', '', $_POST['ha']);
?>
 
<form action="thispage.php" method="post">
<input name="blah" value="<?= $_POST['blah'] ?>" />
<textarea name="ha"><?= $text ?></textarea>
</form>
 
Please explain how this is vulnerable to xss exploits and what I can do to prevent them. The user will be legitimately inputting certain HTML tags, and I don't want these to be ultimately escaped.
User avatar
greyhoundcode
Forum Regular
Posts: 613
Joined: Mon Feb 11, 2008 4:22 am

Re: Securely retaining user input in a form

Post by greyhoundcode »

I think the real vulnerability is SQL injection. I'm not a security expert - be interesting to hear from someone who is - but I understand the mechanics are something like so: let's say you take user input and store it on your database using a statement something like this-

INSERT INTO `table_1` (`field1`) VALUES ('$userInput');

and the string $userInput, collected from your HTML form, contains something like this-

'); DROP DATABASE `user_data`;

Expanding $userInput we can see that what is actually being passed is a new set of statements-

INSERT INTO `table_1` (`field1`) VALUES ('');
DROP DATABASE `user_data`;

I'm sure you get the gist. Here's a solution, or at least part of a solution:

Code: Select all

// Call before you place user input on to the database
function MakeSafe($string)
{
    $string = htmlentities($string);
    $string = addslashes($string);
    return $string;
}
 
// Call when you've retrieved the data and wish to display it
function MakeDisplayable($string)
{
    $string = stripslashes($string);
    $string = html_entity_decode($string);
    return $string;
}
It's also possible of course that legitimate input might contain characters like the apostrophe that could accidentally break your SQL statements, so it's worth having a defensive measure in place anyway.
User avatar
arjan.top
Forum Contributor
Posts: 305
Joined: Sun Oct 14, 2007 4:36 am
Location: Hoče, Slovenia

Re: Securely retaining user input in a form

Post by arjan.top »

MakeSafe does not make the input safe

MakeDisplayable does not prevent XSS

and I see no point in using htmlentitles and then html_entity_decode when displaying
User avatar
greyhoundcode
Forum Regular
Posts: 613
Joined: Mon Feb 11, 2008 4:22 am

Re: Securely retaining user input in a form

Post by greyhoundcode »

The user will be legitimately inputting certain HTML tags
Implied to me that some tags may not be acceptable (since we're discussing attacks perhaps <script> would be one to watch), and even if that isn't exactly what Cut meant I think it's worth consideration.

Like you say, Arjan, the htmlentities/html_entity_decode aspect of the two functions is pretty pointless. But they're just illustrative, and would obviously need to be expanded upon quite a bit. I'm sure you can use your imagination and think up processes that would block certain unacceptable tags.

As for not stopping XSS, you may well be right, I was just trying to think through the problem, or an aspect of the problem, related to the original question about taking user input and making it safe. Sometimes talking through a problem helps to get to the bottom of it - if you know something more why not elaborate on it?
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: Securely retaining user input in a form

Post by Mordred »

greyhoundcode, these functions are crap, as arjan already pointed out. Throw them away and use proper ones.

Moreover, Cut's code doesn't call the database, so how on Earth did you think the problem is SQL injection?!

@Cut: Yes, this code is vulnerable, don't use it. str_replace(), in the way you used it, is trivially exploited (</TextarEa>). Don't try to fix it though, you'll only get it wrong ;). If you want to allow only a select subset of HTML tags, you should be using htmlpurifier. If not - htmlspecialchars with care on all three parameters.
Post Reply