Jump to content

Recommended Posts

Hi Guys

 

I'm on a bit of a security trip at the moment so adopting PDO and generally making sure I am tighter on all aspects (which seems like an endless journey).

 

Can anyone tell me whether echoing a POST value directly back to a form (see example below) exposes any security issues? I can't really think of any reason that it would be a security issue but I would like to know that's the consensus.

 


             <form method="POST" action="#">
	<label>Name:</lable>
	<input type="text" name="name" maxlength="25" value="<?php echo  (isset($_POST['name']) ?  $_POST['name'] : null);?>"/>
	<br/>
	<input type="submit" value="Submit me" />

</form>

 

Thanks,

 

Drongo

Link to comment
https://forums.phpfreaks.com/topic/266494-echo-form-values-and-security/
Share on other sites

If someone made a phishing site that looked like your site and got one of your visitors to goto the phising site instead of your site, and there was a form that submitted to your site's form processing code with XSS code as part of the the actual $_POST['name'] data, that XSS code would be output to and ran in the client's browser, sending any of actual data that your site just output to the client back to the hacker.

 

All external data ($_POST, $_GET, $_COOKIE, $_FILES, and some of the $_SERVER variables) cannot be trusted and can be anything and must be validated/filtered/escaped/type cast...

Thanks PFM!

 

 

 

If someone made a phishing site that looked like your site and got one of your visitors to goto the phising site instead of your site, and there was a form that submitted to your site's form processing code with XSS code as part of the the actual $_POST['name'] data, that XSS code would be output to and ran in the client's browser, sending any of actual data that your site just output to the client back to the hacker.

 

All external data ($_POST, $_GET, $_COOKIE, $_FILES, and some of the $_SERVER variables) cannot be trusted and can be anything and must be validated/filtered/escaped/type cast...

If you remove the maxlength attribute, you can see some XSS in action by entering the following into the name field:

 

"><script>alert('here');</script>

 

 

 

Adding htmlentities() around the POST data would help avoid this issue.

 

<input type="text" name="name" value="<?php echo (isset($_POST['name']) ? htmlentities($_POST['name']) : null);?>"/>

 

 

Plus, the form wouldn't break if someone entered the following for their name:

 

Dan "The Man"

Thanks Cyb

 

I think that'll be a nice clean way to guard against it then.

 

The data is ultimately being passed to a validation class that does strip_tags, trim and  a preg_match to clean and check the data - then if it passes validation I'll use prepared statments with PDO to insert.  So hopefully that ties up the latter stages of the process from a security stand point.

 

Does that sound like fair approach?

 

Obviously none of that guards against XSS but hopefully tweaking the forms I produce as you've suggested can guard against that.

 

 

Thank PFM. Really appreciate the help chaps.

 

Incidentally this is just a test form because ultimately I will be writing a class that goes directly into a backoffice sql database (not mysql). So I was planning to cleanse the data with a preg_replace so i will essentially strip everything - a pattern along the lines of:

 

/^[^a-zA-Z\s\b\'\"]$/

 

Does that sound like a  reasonable way to strip down the inputs?

 

It's the first time I'm doing anything that goes directly into back office systems (i.e. something that's not mysql) - hence my sudden elevated interest in ensuring security is done to the max.

 

 

Is there a business reason you are stripping out anything that is not in that character class list of characters? Because there is no technical/security reason to do so. What about someone with a hyphenated name "Smith-Johnson"? I'm not advocating that you add the hyphen to that character class. What I am saying is that for you to try and determine what all the "valid" characters are (especially for a name) is presumptuous. either you are going to have instances where you are going to piss someone off because you are telling them their name is not valid or you are going to leave the opportunity to submit characters that *could* be a problem if not already handled appropriately. So, the solution is to simply escape/sanitize all data as appropriate for how it will be used. When using in a query use the appropriate function/process for the database. When echoing to an html page use something like htmlentities(), etc.

 

Now, if you have a business reason to exclude certain characters then by all means exclude them. But, you should not strip them out. Because then you would be saving data that is different than the user entered. Just as above, you might think there is no valid reason for a user to use tags in their name, but how do you know someone didn't legally change their name to something stupid such as "<b>John</b>" (with the bold tags). So, instead of stripping it out, do a test. if the input fails the test give the user an appropriate error message and make them fix it.

 

And don't forget about accented characters!(e.g. ?, ?, ?, ?, ?)? Because those aren't covered in your RegEx expression either.

 

Hi Pyscho

 

I definitely take your point and thanks for your answer.

 

There's isn't a 'business' reason as such. It just seemed logical to me that it would be prefereable to end up with data in the database that is as pure as possible (though I grant you that my regex is very shortsighted).

 

Mostly because that sql data may not exclusively be used by php or for web applications. There may be backoffice system  will access the tables too. And as I don't really have much knowledge of how other programming languages deal with sql data it seemed sensible to store the data in as pure form as possible - thereby making it's use in other programs a less issue prone process.

 

I may of course - be totally and completely off the mark :/

 

If you were capturing data on the web, that may be ultimately be used by any number of other languages, what would you suggest is the most sensible way is to cleanse the data?

 

And apologies - my ignorance might be over complicating this.

 

 

 

I always validate all user input, without exceptions. As the first thing I do, after I've validated that the user has sent input. Anything less, and I've potentially opened the doors for an attacker.

Rule 0: Never underestimate the dark side.

 

When it comes to sending output to other systems (web browsers, shell, file systems, DB engines, etc), I always escape output. Regardless of whether or not I know of any potential manner in which to exploit it. Even if I believe it to be completely secure, there is always someone smarter or more knowledgeable than me. (See rule #0.) ;)

Once you've done this, you should be reasonably secure from attackers.

 

As for it being presumptuous to try to validate names, I don't agree. There are certain basic patterns to names that's more or less globally available, and by following these rules you should be fairly certain you accept the vast, vast majority of names out there.

This is the function I use to validate names, from a collection of validation functions, and I've yet to see anyone complain about it:

/**
* Validates that string contains only characters legal for names.
* Optionally extra characters can be selected, as well as max length.
*
* @param string $String
* @param string $Extra
* @param mixed $MaxLength
* @return mixed
*/
function Val_name ($String, $Extra = '', $MaxLength = '+') {
if ($MaxLength == "*" && $String == '') {
	return '';
}

if (is_int ($MaxLength)) {
	$MaxLength = "{1,$MaxLength}";
} elseif ($MaxLength != "*") {
	$MaxLength = '+';
}

$OKChars = addcslashes ($Extra, '."!?\'*|$[]<>%#^/\\').'\\w\\pL \\.\\-';

if (preg_match ('/^[a-zA-Z\\pL]['.$OKChars.']'.$MaxLength.'\\z/u', $String)) {
	return $String;
}

return false;
}

 

It accepts all letters, latin and other locales, space, period and hyphen. It does not allow numbers, as no-one (to my knowledge) use actual numbers in their name. Those that have title "the Third" (or similar) writes it out like that, meaning it falls within the realm of acceptable characters.

 

PS: Validation and stripping (washing) input are two different things, as Psycho stated above. Generally you will always want to use validation and not stripping for security testing.

One other question Christian

 

If you run a function that escapes data like that, then push it through PDO prepared statements, would you end up double escaping the data?

 

 

 

I always validate all user input, without exceptions. As the first thing I do, after I've validated that the user has sent input. Anything less, and I've potentially opened the doors for an attacker.

Rule 0: Never underestimate the dark side.

 

When it comes to sending output to other systems (web browsers, shell, file systems, DB engines, etc), I always escape output. Regardless of whether or not I know of any potential manner in which to exploit it. Even if I believe it to be completely secure, there is always someone smarter or more knowledgeable than me. (See rule #0.) ;)

Once you've done this, you should be reasonably secure from attackers.

 

As for it being presumptuous to try to validate names, I don't agree. There are certain basic patterns to names that's more or less globally available, and by following these rules you should be fairly certain you accept the vast, vast majority of names out there.

This is the function I use to validate names, from a collection of validation functions, and I've yet to see anyone complain about it:

/**
* Validates that string contains only characters legal for names.
* Optionally extra characters can be selected, as well as max length.
*
* @param string $String
* @param string $Extra
* @param mixed $MaxLength
* @return mixed
*/
function Val_name ($String, $Extra = '', $MaxLength = '+') {
if ($MaxLength == "*" && $String == '') {
	return '';
}

if (is_int ($MaxLength)) {
	$MaxLength = "{1,$MaxLength}";
} elseif ($MaxLength != "*") {
	$MaxLength = '+';
}

$OKChars = addcslashes ($Extra, '."!?\'*|$[]<>%#^/\\').'\\w\\pL \\.\\-';

if (preg_match ('/^[a-zA-Z\\pL]['.$OKChars.']'.$MaxLength.'\\z/u', $String)) {
	return $String;
}

return false;
}

 

It accepts all letters, latin and other locales, space, period and hyphen. It does not allow numbers, as no-one (to my knowledge) use actual numbers in their name. Those that have title "the Third" (or similar) writes it out like that, meaning it falls within the realm of acceptable characters.

 

PS: Validation and stripping (washing) input are two different things, as Psycho stated above. Generally you will always want to use validation and not stripping for security testing.

Hehe, just to make sure there are no misunderstandings:

Validation stops and throws an error/warning when you encounter invalid content, stripping just silently removes it.

 

If you're referring to the Val_name () function, then it does not escape output. It's a input validation function. Escaping output is adding characters to the data, to ensure that any (potential) meta-characters of the external system are rendered harmless. It has nothing to do with input validation, other than both being security-related.

However, if you did manually escape the output and then pushed it through prepared statements you might1 very well end up with double-escaped data. Yes. So that's not recommended. :P

 

1"Might" because I've never actually tried it myself.

One other question Christian

 

If you run a function that escapes data like that, then push it through PDO prepared statements, would you end up double escaping the data?

 

Not Christian, but no, as that function does not escape the data.  There's a difference between validating and sanitizing data, and escaping data.

 

Validation and sanitation are about whether or not the data is the right kind of data.  Escaping is of a more mechanical nature - it only transforms (escapes) anything in the incoming data that would otherwise be handled specially by the DB.  For example, single quotes and semicolons have a particular functional meaning in SQL, so they need to be escaped to be treated as mere normal text.

There's isn't a 'business' reason as such. It just seemed logical to me that it would be prefereable to end up with data in the database that is as pure as possible (though I grant you that my regex is very shortsighted).

 

Mostly because that sql data may not exclusively be used by php or for web applications. There may be backoffice system  will access the tables too. And as I don't really have much knowledge of how other programming languages deal with sql data it seemed sensible to store the data in as pure form as possible - thereby making it's use in other programs a less issue prone process.

 

I may of course - be totally and completely off the mark :/

 

If you were capturing data on the web, that may be ultimately be used by any number of other languages, what would you suggest is the most sensible way is to cleanse the data?

 

In my opinion, keeping the data "pure" is keeping it EXACTLY as entered. For example, I see some people using htmlentities() on data before storing it in the database. But, htmlentities() is designed to safeguard data being displayed in an HTML page. If the data needed to be output into a text file or some other format it could be made unreadable because of the translation of the data.

 

So, I would always advise not excluding data unless there is a need to. Only escape/cleanse the data as needed based upon the specific output/usage. Different "languages" don't have any particular issues with specific data that I am aware of - it is HOW the data is used. You can always make a determination when the other processes/languages are implemented to determine what procedures are needed to safeguard against possible data issues. Besides, the hyphen or apostrophe could *potentially* cause issues within some processes, but it wouldn't make sense to exclude those for a name field.

 

Again, my opinion, is to only reject content when there is a legitimate business need (e.g. no letters in a phone number). Then escape/sanitize the data as appropriate based upon the usage/output. The only time I would strip out content without the user's knowledge would be something like a phone number. I would strip out the formatting characters (periods, spaces, parents, etc.) and store only the digits of the phone number. That way I could display the phone number in a consistent format during output.

One other question Christian

 

If you run a function that escapes data like that, then push it through PDO prepared statements, would you end up double escaping the data?

 

Not Christian, but no, as that function does not escape the data.  There's a difference between validating and sanitizing data, and escaping data.

 

Validation and sanitation are about whether or not the data is the right kind of data.  Escaping is of a more mechanical nature - it only transforms (escapes) anything in the incoming data that would otherwise be handled specially by the DB.  For example, single quotes and semicolons have a particular functional meaning in SQL, so they need to be escaped to be treated as mere normal text.

 

I'd say arguably escaping is a form of sanitation, but I agree that validation is quite different.

 

And for semantics, double and single quotes are interchangeable in most major SQL variants. Both should be and generally are escaped. Semi-colons should never need to be escaped either. In a SQL string, they mean nothing and don't need escaping. Outside of a string, they shouldn't exist (assuming a single query), and should be removed rather than escaped.

Thank you to everyone who has commented on this. It has really helped loads.

 

Psycho - i totally see what you mean, especially about sending htmlentity data into the database.

 

So to summarise with an example.

 

If i was going to store the data from a 'name' field I would very simply have to just:

 

[*]trim the data

[*]run a regex along the lines of /^[a-zA-Z\-\.\'\s\b]{2, 25}$/ - just to ensure it's roughly along the lines of what I need and that nothing majorly bad is likely to be used in the rest of the application

[*]type cast the variables where appropriate

[*]Use prepared statement to store the data

 

Then upon retrieval from the database manipulate it as is required. So if it's going to the browser ensure the data is htmlentitied.

 

For my sanity and moral - would anyone add anything to this or is that about right and a safe way to go?

 

 

 

 

 

There's isn't a 'business' reason as such. It just seemed logical to me that it would be prefereable to end up with data in the database that is as pure as possible (though I grant you that my regex is very shortsighted).

 

Mostly because that sql data may not exclusively be used by php or for web applications. There may be backoffice system  will access the tables too. And as I don't really have much knowledge of how other programming languages deal with sql data it seemed sensible to store the data in as pure form as possible - thereby making it's use in other programs a less issue prone process.

 

I may of course - be totally and completely off the mark :/

 

If you were capturing data on the web, that may be ultimately be used by any number of other languages, what would you suggest is the most sensible way is to cleanse the data?

 

In my opinion, keeping the data "pure" is keeping it EXACTLY as entered. For example, I see some people using htmlentities() on data before storing it in the database. But, htmlentities() is designed to safeguard data being displayed in an HTML page. If the data needed to be output into a text file or some other format it could be made unreadable because of the translation of the data.

 

So, I would always advise not excluding data unless there is a need to. Only escape/cleanse the data as needed based upon the specific output/usage. Different "languages" don't have any particular issues with specific data that I am aware of - it is HOW the data is used. You can always make a determination when the other processes/languages are implemented to determine what procedures are needed to safeguard against possible data issues. Besides, the hyphen or apostrophe could *potentially* cause issues within some processes, but it wouldn't make sense to exclude those for a name field.

 

Again, my opinion, is to only reject content when there is a legitimate business need (e.g. no letters in a phone number). Then escape/sanitize the data as appropriate based upon the usage/output. The only time I would strip out content without the user's knowledge would be something like a phone number. I would strip out the formatting characters (periods, spaces, parents, etc.) and store only the digits of the phone number. That way I could display the phone number in a consistent format during output.

And for semantics, double and single quotes are interchangeable in most major SQL variants. Both should be and generally are escaped. Semi-colons should never need to be escaped either.

 

Isn't this a contradiction?

 

Haha, was lazy with the way I was writing that.

 

Semi-colons aren't escaped in most SQL languages. They should exist, beyond being inside of a string (no escape) or terminating queries (no escape).

 

Both double and single quotes should be escaped. The provided SQL escape functions (mysqli::escape_string for example) will escape both.

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.