Sanitisation

[Orangutang]

Hi,

Based on what I've read to date I've put together my first sanitisation process, I'm not sure if I've done too much or too little and I'm hoping someone could
help me review it to either point out my errors or help me improve it.


1. I pick up the posted username with the following:

if (isset ($_POST['username']) and ! empty ($_POST['username'])){


2. Then strip the html tags with:

$username = strip_tags($_POST['username']);


3. Then pass it to a sanitisation function:

$username = san_data_un();

function san_data_un() {
GLOBAL $username;
$username = trim($username);
$username = preg_replace('/[^A-Za-z0-9\._-]/', '', $username); // Leave A-Z, a-z, 0-9, dots, spaces, underscores and hyphens
$username = preg_replace('/ */', '-', $username); // Replace spaces with hypens
$username = htmlspecialchars("$username", ENT_QUOTES, 'utf-8');
return $username;
}


4. When it comes out of the function I check its type with:

if (!is_string($username)) {
echo "ERROR 1000";
header("Location: h_sign_in.php");
exit(0);
}


5. I then use a prepared statement to communicate with the db, check the details and retrieve other data needed.


6. Then put this other data in an array with:

$main_data = array("$var_1", "$var_2", "$var_3");


7. Then prepare it to send via the URL with:

$coded_data = urlencode(serialize($main_data));


8. Then pass it like:

<a href="next_page.php?pass_main_data=<?php echo($coded_data);?>">Link</a>


Thanks in advance for any help or suggestions.

[Readie]

Your script could be made to throw a warning by somebody posting an array to it. Since all POST data is either a string, or an array of strings, you should perhaps make your first if() include: " && is_string($_POST['username'])"

[Orangutang]

Thanks Readie,

Thought there'd be a quite a few errors so just the one is good news. The new if() is:
if (isset ($_POST['username']) and ! empty ($_POST['username']) && is_string($_POST['username'])) {

Following on from this the next page is:

1. I pick up the data sent via the URL with:

if (isset($_GET['pass_main_data']) and ! empty ($_GET['pass_main_data']) && is_string($_GET['pass_main_data'])) {

$coded_data = unserialize(urldecode($_GET['pass_main_data']));


2. Loop through the array to break out the individual vars. (These vars are from the db and they are all numbers as they were not user generated but inserted as a number in a prepared statement.)

foreach($coded_data as $key => $val) {
$var_1 = "$main_data[0]";
$var_2 = "$main_data[1]";
$var_3 = "$main_data[2]";
}


3. Then I sanitise these with: (All three vars are numbers and I do exactly the same with all three.)

$var_1 = san_data_var_1();

function san_data_var_1() {
GLOBAL $var_1;
$var_1 = strip_tags($var_1);
$var_1 = trim($var_1);
$var_1 = preg_replace('/[^0-9]+$/', '', $var_1); // Leaves numbers only
$var_1 = htmlspecialchars("$var_1", ENT_QUOTES, 'utf-8');
return $var_1;
}
Q1: But I'm not sure if I need to do this as they are internal and I know there numbers as stated above?


4. When it comes out of the function I check its type with:

if (!is_numeric($var_1)) {
echo "ERROR 1000";
header("Location: h_sign_in.php");
exit(0);
}
Q2: Again unsure if I need to do this as it's just the second part of the sanitisation process which started above in 3.


5. Then use the vars and then put them into hidden inputs in a form with:

<form method="POST" action="another_page.php">
<input type="hidden" name="z_var_1[<?php echo $var_1;?>]" value="">


6. Then pick them up with:

if ($_POST['z_var_1']) {
$z_var_1_array = isset($_POST['z_var_1']) && is_array($_POST['z_var_1'])? $_POST['z_var_1']: array();
foreach(z_var_1_array as $var_1 => $key) {
}
}

And then I have $var_1 to use again.

Q3: And again unsure if I need to sanitise them again at this point with exactly the same function and method as stated in point 3 and 4 above?


Hopefully you can see where I'm unsure, in summary would you recommend I sanitise them at every point or am I repeating what I've already done and in reality achieving nothing?

[robzilla]

After...

$username = preg_replace('/[^A-Za-z0-9\._-]/', '', $username);

...what's the point of...

$username = htmlspecialchars("$username", ENT_QUOTES, 'utf-8');

?

$username = preg_replace('/[^A-Za-z0-9\._-]/', '', $username); // Leave A-Z, a-z, 0-9, dots, spaces, underscores and hyphens
$username = preg_replace('/ */', '-', $username); // Replace spaces with hypens

If you want to replace whitespace with the second line, you should probably include \s in the regular expression of the first line. Right now, it seems your second line would try to replace something (whitespace) that the first line does not allow (and has replaced).

Incidentally, for something as simple as replacing one character with another (i.e. spaces with hyphens), str_replace(' ','-',$username) should suffice.

Your preg_replace method allows for more user error within PHP, but I generally prefer to validate user input with Javascript before I send it off to PHP, where a strict preg_match against the input makes sure there's no funny business. This allows me to alert the user about incorrect input before bothering PHP with it.

function san_data_un() {
GLOBAL $username;

Why a global variable when you can pass $username to the function, like so: function san_data_un($username) {}

[Orangutang]

Hi robzilla,

Thanks for the reply, your advice is exactly what I needed.

What's the point of:
$username = htmlspecialchars("$username", ENT_QUOTES, 'utf-8');

Understood, wasn't sure if having backup sanitisation was needed, now i know it isn't.

////////////////////////////////

function san_data_un() {
GLOBAL $username;

Didn't want to use a Global but when I tried to pass the variable like below I couldn't get it to work.

function san_data_un($username) {}

Will try that again as not sure what happened there?

/////////////////////////////////

Your other advice about replacing one character and using js, thanks.

/////////////////////////////////

My other concern due to not knowing very much about hacking is the following;

Say for instance:

1. I use a prepared statement to insert the number 12345 into a mysql db.

2. I then use a prepared statement to retrieve this number.

3. I then loop through the result to free up the number and store it in a var called $var_1

4. I then put $var_1 into a hidden form field and use POST to send it to the next page.

5. I then pick up $var_1 on the next page and free it up ready to use.


My question is would I need to do any sanitisation at all using the scenario above, is it possible for this data to be corrupted in any way by a hacker?

[robzilla]

4. I then put $var_1 into a hidden form field and use POST to send it to the next page.

is it possible for this data to be corrupted in any way by a hacker?

A 'hidden' form field isn't actually hidden, though, is it? It's right there in the HTML source of your page, and, more importantly, it's there in the HTTP headers of the POST request, which everyone and their grandmother can alter. So, yes, it is in fact quite easy to corrupt the data that you pick up in your 5th step.

Didn't want to use a Global but when I tried to pass the variable like below I couldn't get it to work.

function san_data_un($username) {}

Will try that again as not sure what happened there?

I'd wrap the whole sanitization process up into a single function, like so:

function san_data_un($data) {

// Your regex magic goes here (which in this example you apply to $data, not to $username)

}

and then you call it when needed, like so:

$username = san_data_un($_POST['username']);

due to not knowing very much about hacking

The most important thing to remember -- the golden rule of sanitization, if you will -- is that you can never trust the client.

[Orangutang]

I think I'm making some progress.

I've looked at the source code and I see what you mean, all there in black and white. What I found even more strange was even though my code looks like:
<input type="hidden" name="userid[<?php echo $user_id;?>]" value="">

When I viewed the source code it displayed:
<input type="hidden" name="userid[201]" value="">
It actually showed the content of the variable and I thought anything within php tags was hidden.

I've never looked at the HTTP headers before. I had a quick try at changing them but I think I need some additional software or plug-in, I'll have to save that for another day.

//////////////////////////////////////////////////////////

Thanks for picking up on the use of a GLOBAL in the function by the way, as per advice changed that now, works great.

I've put together 2 new functions, one for the username and password. I think they've improved?

I enter: \^$._-|()*+{}%?*123 "&<>£<b>He llo</b>
// This function sanitises the username on login and register
function san_data_un($data) {
$data = strip_tags($data);
$data = trim($data);
$data = preg_replace('/[^A-Za-z0-9._-\s]/', '', $data); // Leave A-Z, a-z, 0-9, dots, underscores, hyphens and spaces,
return $data;
}
Output is ._-123 He llo

I enter: \^$._-|()*+{}%?*123 "&<>£<b>He llo</b>
// This function sanitises the password on login and register
function san_data_pw($data) {
$data = strip_tags($data);
$data = trim($data);
$data = preg_replace('/[^A-Za-z0-9%?*._-\s]/', '', $data); // Leave A-Z, a-z, 0-9, %, ?, *, dots, underscores, hyphens and spaces,
return $data;
}
Output is ._-*%?*123 He llo

As a note, I notice even though there are 3 spaces after the number 3 and another 3 spaces after the e it automatically reduces them down to 1 space?

Well there were 3 spaces but after pressing submit it's reduced them down to 1 on here as well?

///////////////////////////////////////////////////////////

The most important thing to remember -- the golden rule of sanitization, if you will -- is that you can never trust the client.

In summary if I combine this advice with the advice above about the HTTP headers it means that changes can be made if any one of the following occur:
a page is refreshed, a submit button is pressed or a hyperlink is clicked.

Which means I should sanitise and validate in accordance with the above occurring.

[edited by: Orangutang at 4:06 pm (utc) on Oct 26, 2013]

[Orangutang]

On the basis of the above and hopefully identifying what I need to sanitise and validate and when I've documented the flow of the data around my application. I appreciate there a lots of other functions like strlen to use but I'm hoping you could look at what I've done and let me know if what I've written is correct.

(Especially where I've written DONT NEED TO SANITISE OR VALIDATE AT THIS POINT.)

// Scenario 1
1. User input from a form = Sanitise using strip_tags, trim, preg_replace(regular expression), str_replace or JS.
2. Inserting this data into a db = Use prepared statement.
3. Selecting this data from a db = Use prepared statement and validate using either is_string, is_numeric, is_float, is_bool, is_array or is_object.
4. Free up the data ready to use either as individual or with loop.
5. DONT NEED TO SANITISE OR VALIDATE AT THIS POINT.
6. Can use this data like: if (check_superuser($var_1)) {...
7. Can put this data into a hidden from field.
8. Can put data into URL to send.
9. Can display this data to user = Sanitise using htmlspecialchars.

// Scenario 2
1. Not user generated - I stipulate the value and insert data into a db = Use prepared statement.
2. Selecting this data from a db = Use prepared statement and validate using either is_string, is_numeric, is_float, is_bool, is_array or is_object.
3. Free up the data ready to use either as individual or with loop.
4. DONT NEED TO SANITISE OR VALIDATE AT THIS POINT.
5. Can use this data like: if (check_superuser($var_1)) {...
6. Can put this data into a hidden from field.
7. Can put data into URL to send.
8. Can display this data to user = Sanitise using htmlspecialchars.

// Scenario 3
1. Passing variables in a hidden form field and use POST to send it to next page.
2. On next page use isset, empty and either is_string, is_numeric, is_float, is_bool, is_array or is_object.
3. Free up the data ready to use either as individual or with loop.
4. DONT NEED TO SANITISE OR VALIDATE AT THIS POINT.
5. Can use this data like: if (check_superuser($var_1)) {...
6. Can put this data into a hidden from field.
7. Can put data into URL to send.
8. Can display this data to user = Sanitise using htmlspecialchars.

// Scenario 4
1. Passing variables via a URL = Use urlencode and serialize.
2. On next page use isset, empty and either is_string, is_numeric, is_float, is_bool, is_array or is_object.
3. Then use unserialize and urldecode.
4. Free up the data ready to use either as individual or with loop.
5. DONT NEED TO SANITISE OR VALIDATE AT THIS POINT.
6. Can use this data like: if (check_superuser($var_1)) {...
7. Can put this data into a hidden from field.
8. Can put data into URL to send.
9. Can display this data to user = Sanitise using htmlspecialchars.

Really appreciate all your help with this as I've read so many times how important it is to sanitise and validate data but to date all I have known is what I don't know.

[robzilla]

Again, what's the point of

$data = strip_tags($data);

when it's followed by

$data = preg_replace('/[^A-Za-z0-9._-\s]/', '', $data); // Leave A-Z, a-z, 0-9, dots, underscores, hyphens and spaces,

?

I thought anything within php tags was hidden.

Well, yes, until you print it to the page via echo or other means. You see "userid[<?php echo $user_id;?>]" in your source code, but that's not what the client sees.

[penders]

Bit late to the thread, and admittedly not read it all, but... it would be useful to state in natural language what you are trying to achieve before presenting a code block to analyse. You appear to be sanitising a "username"? My first query is why do you need to sanitise a username?

if (isset ($_POST['username']) and ! empty ($_POST['username'])){

Minor point, you don't need to check isset() before checking empty().

[Dinkar]

Keep it Simple, Stupid.

Sometimes it's better to know what you need than what you don't need.

If you know that you need (A-Z and 0-9) for the username then you don't need to check anything else like special char or html tags etc etc.

I don't know why people do such checks when there is no need.

[Orangutang]

Robzilla,

Thanks for the advice, I'm using strip_tags before the preg_replace because if I didn't the b's from the <b></b> are left and I wanted to eliminate the html tags.

Interesting second point, thanks for letting me know.

/////////////////////////////////////////////////////

Penders,

Thanks for the advice. In the future I will try to explain before I post a code block.

The reason I'm sanitising the username is because I only want to allow certain chars, for instance I don't want someone to have a username of <b>un</b>, maybe I'm wrong to not want this but as yet my knowledge about how php special chars and html tags can effect the code as the sripts run is very limited so to be sure I thought I'd do more rather than less.

Hopefully as I learn more I can code better but I have given myself until Jan 14 to lauch my first application which negates the possibility of me learning everything I want to the degree I want, I was hoping that you guys could help put my mind at rest in the area's I'm weak on so I could be confident that the code I use when I launch is good enough.

What I have learnt is that I will get an experiened php programmer to review my code before I launch, I think I can afford this as I know I couldn't afford to get the whole system done.

Minor point, you don't need to check isset() before checking empty(). Thanks, another one for me to learn more about.

////////////////////////////////////////////////////

Thank you both for trying to help but I think I need to learn more about how php special chars and html tags can effect the code as the sripts run. Also about how the code can be altered whilst it moves around within the page and moves to the next page.

////////////////////////////////////////////////////

Dinkar,

Over the weekend a friend told me my ex had arranged a post on here, she is so bitter!

I didn't read the rest of your post because I think what would be stupid is to listen to anything to say!

[penders]

Dinkar is on the button, "Keep it Simple, Stupid." - the "KISS principle [en.wikipedia.org]".

The reason I'm sanitising the username is because I only want to allow certain chars, for instance I don't want someone to have a username of <b>un</b>

You should be validating, not sanitising the username in this instance. If the username fails validation, simply reject it. Sanitising a value could involve changing that value. There is not usually any need to change a username. Other forms of submitted data, such as messages posted in a contact form, then yes that could be sanitised.

As Dinkar mentions, the format of a username is usually very concise, consisting of just the letters [a-zA-Z_0-9] (a regex class) and perhaps between 4 and 20 characters long.

$username = isset($_POST['username']) ? $_POST['username'] : null; 
if (isset($username)) { 
  if (preg_match('/[a-zA-Z_0-9]{4,20}/',$username)) { 
    /* Username looks OK */ 
  } else { 
    /* Reject username */ 
  } 
} else { 
  /* Username has not been submitted */ 
}

[Orangutang]

I apologise, I understand now the KISS principle.

Thanks for clearing that up and your post. Will implement it and try to learn more about it.

(The advice I received from friend at the weekend, very strange.)

Thanks again.

[penders]

I'm curious, what advice did your friend give you?

[londrum]

if it was me, i would probably check the string length as well. usernames don't have to be that long, so anything longer than 12, or whatever (and shorter than 3?), can automatically be rejected

even if your form stops them from entering too many letters, that can be got around pretty easily by them just writing their own form and sending it your URL to be processed

[Orangutang]

The advice was by email and read, you will be made to look like a fool on WebmasterWorld. From Joe.

Then after your post I went up the shop and on the way there a women said easy peezie as she walked past then on the way back I had another women say easy peezie as she walked past.

I've walked up the shop at least 200 times and never had 2 people say that as they walked past, and they weren't even talking on a phone?

A coincidence, never in a million years in my opinion.

[penders]

The advice was by email and read, you will be made to look like a fool on WebmasterWorld.

lol - I thought it was something constructive! No ones a fool; we're all in it to learn. :)

Just a point regarding the "sanitising" of a username. You would sanitise a username if you were outputting a rejected username back to the user (in the case of a new registration request in an HTML form). But even then, you would only need to htmlentities() the username, and perhaps truncate its length.

[Orangutang]

:-)

The sanitising of the username, great I now understand how to put all that together.

As a note:

I've just spoken to Joe and she said she never sent me an email!

So I looked at the email again and even though it says it's from Joe ..... I don't think it is, it's got spammy stuff below the message.

It basically looks exactly like a spam email does apart from it says it's from Joe .... which is a contact in my addy book and at the start its got that sentence written.

I'm still sure it's that dam ex and her friends causing problems :-)

[Orangutang]

Hi londrum,

Thanks for the input, I will add a string length check because I'm trying to make the application as secure as possible, mainly because I know what this ex is like :-)

The second part of your post about sending a complete form to my URL, wow I can't say I understand how that's done or how exactly it can compromise my code but it does make me know that I should take at least a couple of weeks out to learn more about how code can be changed by an outside source.

[SevenCubed]

I've just spoken to Joe and she said she never sent me an email!

The plot thickens. I luv a good mystery.

[Orangutang]

Hi SevenCubed,

I luv a good mystery myself but all I know at this point is:

There is a female with what seems like a ridiculous amount of friends intent on playing a mind game with me and it has been going on for several years! (And most of the friends are women aged between 40 and 80) Weird or what :-)

Nothing serious, always only cretin like mind game stuff.

Interesting it is but what exactly their trying to achieve I don't know? This is the ultimate question I have.

In my opinion two heads are always better than one, any ideas would be well received.

At a guess it's my ex belonged to the WI (Womens Institute) and she's lied to them to facilitate her own goal. Not sure if that's what it is but I think it's something like that.

I know one thing for sure, my ex was a real piece of work which is why I ended any contact with her. Nothing would really surprise me about her.

[londrum]

The second part of your post about sending a complete form to my URL, wow I can't say I understand how that's done or how exactly it can compromise my code...

people often assume that the only information they will ever receive at their processing page, is the stuff that comes from their form. but anybody can write a form, put it on any website, and then send the output to your processing page. so none of the checks on your form are foolproof. it's all about how you handle the stuff that you receive.

(of course the chances of someone actually taking the time to do that is very small, but its something to be aware of if you want to make it as secure as possible)

[SevenCubed]

In my opinion two heads are always better than one, any ideas would be well received.

Well not much to go on here so far 'cept that it's almost certain that Joe probably doesn't have hairy legs.

[Orangutang]

@londrum,

That is fascinating, write a form and put it on my website and send the output to my processing page. This is something that I'm going to have to try over the next couple of weeks.

@SevenCubed,

I'm with you on that one :-)

[penders]

...I will add a string length check because I'm trying to make the application as secure as possible

Just to note, the regex (regular expression) I posted above ("/[a-zA-Z_0-9]{4,20}/") includes a string length check. In this case between 4 and 20 chars (inclusive).

(of course the chances of someone actually taking the time to do that is very small...

Not so small. Most spam that is submitted via what "looks-like" your online form is actually submitted by automated bots that will have scanned your form initially to see what sort of data is expected. Subsequent "form submissions" don't go anywhere near your form, or any "HTML form" for that matter. The appropriate HTTP request is sent directly to the URL that processes your form submissions. The important thing to realise is that this process is entirely automated, there may well be very few "real" people "taking the time" to spam your form.

[SevenCubed]

The appropriate HTTP request is sent directly to the URL that processes your form submissions.

That's why it's important to add another barrier between your mailbox and the spammers. Most contact forms are typically on a single page (or at least should be for good reason).

I simply parse for referring page and if it didn't come from the only page on the site that contains the form it doesn't get through.

This short snippet is an example contained at the top of the processing script page that greets a submission after it first gets sanitized by JavaScript, then passes a math quiz without user input or awareness required (hidden fields), on the contact page itself.

<?php
$origin="https://www.example.com/contact.html";
$referral=$_SERVER["HTTP_REFERER"];
$refervalid=0;
if($referral==$origin) $refervalid=1;
if((!$refervalid) OR ($_POST["validated"]!=343)){
echo '<h3><span style="color:#ff0000;">Your Message Was NOT Sent</span>. For Validation Purposes This Form Requires A Valid HTTP Referrer And Referrer Headers Enabled. There Was An Invalid Referrer Or You Are Using A Browser Plugin That Disables It Please Enable It, Use Your Browser Back Button, And Resubmit Your Request.</h3>';
exit;
}
?>

Zero requirement to burden visitors with captcha while at the same time being very effective.

I just did a search to see if I was revealing too much in the output message string and see that I previously posted this elsewhere on WW (but don't remember, hmmmmm, note to self, more coffee needed).

[penders]

I simply parse for referring page ...

Bare in mind that it is easy to fake the HTTP referer. An automated bot that has already scanned the page containing the form will most probably have recorded the URL of that page, and this will be part of the HTTP request sent in the "fake" form submission.

[Orangutang]

Thank you for all the advice,

A lot for me to read up on, a form being put on my website, automated bots, HTTP requests, regular expressions, HTTP Referrer, Referrer Headers and the barrier between the mailbox and spammers.

This is great and thanks again for pointing me in the right direction as to what I need to learn.

[SevenCubed]

Agreed Penders. What I posted above was just a watered down portion of the script. I snipped out all of the referrer data being passed to the processor as well as much of technical quiz that follows if it manages to pass the above 2 tests. I just didn't want to post the whole thing here to prevent feeding the potential spammers.