verifying and displaying DB data securely

[ajoo]

Hi, 

 

Below is how I am handling the database data before I display it on a page. 

.
$query = "SQL QUERY to retrieve some data";
.
.
while ($stmt->fetch())
{ 

$fname = html_escape($fname);
$lname = html_escape($lname);
$city = html_escape($city);
$cell = html_escape($cell);

// verify that $xid is numeric. 
if(($xid = fcheckNumber($xid)) === false) die('Internal error. Conatct Admin';

// verify that $role has a valid value against a set of values.
if(($role = html_escape(fcheckRole($role)))=== false) die('Internal error. Conatct Admin';

// verify that $email is correctly formatted as an email should be. 
if(($email = html_escape(fcheckEmail($email)))=== false) die('Internal error. Conatct Admin';

// verify that $status is numeric. 
if(($status = fcheckNumber($status))===false) die('Internal error. Conatct Admin';

.
.
.

display the above data in a form.

} 

 

My questions are:

 

Is this the right way of handling the data before I display it on a form or am i overdoing it with all the checks and die statements? 

 

Am I missing out some other security aspect here ?

 

 

Then there are instances where i use verify a SESSION variable or a POST / GET variable similarly.

if(($xid = $_SESSION['xid'])===false) die('Internal Error. Contact Admin');
OR
if(($xid = $_POST['id'])===false) die('Internal Error. Contact Admin');

Is this alright or can I skip some of these checks ?

 

 

I'd like to mention here that I use prepared statements for all queries and the same data verification as above when I add the data to the database. I do not html escape any data that is put into the DB.

 

Thanks all !

[benanamen]

Since you verified/validated the data going in, it seems redundant to do it on the way out. If the data changed while it is is sitting in the DB you probably have much bigger problems.

[ginerjm]

while ($stmt->fetch())

 

Where exactly are you 'fetching' the data to? Normally you assign the fetch to an array var and then use that array for values of a row.

[Stratadox]

Interesting question.

 

Is this the right way of handling the data before I display it on a form or am i overdoing it with all the checks and die statements?

Probably, yes. The overdoing part, that is.

The first question in digital security is what you're protecting yourself against.

 

In the case of reading a database, there's three main risks to avoid:

1) Manipulation of the input query

2) Unauthorised viewing of restricted information

3) Attacks on your users, for example stored xss attacks

 

The main defences against those risks are:

1) Using parametrised queries (which you seem to be doing)

2) Using a role/permission system

3) Escaping manipulatable data

 

Any more than that probably doesn't add much.

 

If the checks you propose catch anything, it's already too late anyway.

 

Am I missing out some other security aspect here ?

IMO, the biggest risk in web development is not untrusted user input.

The biggest risk is code that looks all right but that doesn't do what it seems to do.

 

An example of such code is, ironically, those checks you mention.

 

They look secure, because they talk about html escaping and checking roles.

But unless I'm mistaking, they don't actually do any of that.

 

Let's check a check.

 

if(($email = html_escape(fcheckEmail($email)))=== false) die('Internal error. Conatct Admin';

I can't be 100% sure, because I don't have a way of seeing html_escape or fcheckEmail, but I don't believe this will ever catch anything.

 

The moment html_escape returns a string, which it probably will, the result of the assignment to $email will not be false.

Therefore, even invalid emails would be saved to the database and displayed to the user.

 

The best (and only reliable) way to mitigate the risk of allright looking but not actually working code is automated testing at every level.

 

By testing each unit of code, and preferably each integration of units, in an automatic way, you can prevent these kinds of bugs.

 

Check out tools like PhpUnit, Codeception or Behat.

It may take a bit of extra time to write tests (writing good tests can be quite a challenge)

However, in terms of security, it's one of the most important (and probably one of the most overlooked) aspects.

 

Good luck!

Edited January 22, 2018 by Stratadox

[ajoo]

Hi !! 

 

Thanks benanamen, ginerjim and stratadox for the response. 

 

@benanamen: 

 

 

Since you verified/validated the data going in, it seems redundant to do it on the way out.

 

That's one of the reasons that I asked this question. However, I do recall reading somewhere, data should never be trusted even if it is coming from a DB. I believe there are cases when the data of a particular user may be compromised and as against the entire DB. In such cases, these checks will restrict attacks, that would otherwise occur, like stored xss attacks as mentioned by stratadox.

 

@ginerjim:

 

I am fetching the data directly into the variables as shown. I don't need the array as the data is displayed as an HTML page immediately after the data validation. I am using the fetch to directly populate the data variables. 

 

@stratadox:

 

 

3) Attacks on your users, for example stored xss attacks

Yes, That's what I am seeking to avoid by these checks.

 

 

 

I can't be 100% sure, because I don't have a way of seeing html_escape or fcheckEmail, but I don't believe this will ever catch anything.

 

I have extensively checked fcheckEmail and html_escape function. html_escape was provided by Guru Jacques, so can't be wrong. Here's the code for these below for you to verify.

 

function fcheckEmail($str) // to validate the format of an email.
{
$str = filter_var($str, FILTER_VALIDATE_EMAIL);
return $str;} 

I understand the issues with filter_var but my intent here is to check that the data passed by the user is in the valid format. 

function html_escape($raw_input)
{
    return htmlspecialchars($raw_input, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
}

 

 

The moment html_escape returns a string, which it probably will, the result of the assignment to $email will not be false.
Therefore, even invalid emails would be saved to the database and displayed to the user.

The best (and only reliable) way to mitigate the risk of allright looking but not actually working code is automated testing at every level.

 

I think the above stems from the fact that you are assuming that the functions do not do what they say they do. So I have put in the snippet functions for you to see and check if you would like to.

 

 

 

However, in terms of security, it's one of the most important (and probably one of the most overlooked) aspects.

 

hmmm I know about unit testing but yes I have not incorporated that yet. Will revisit that sometimes in the near future again.

 

Thanks for a reference to the tools for unit testing. 

 

 

Thanks all !!

 

[Psycho]

Here is my advice:

 

1. Never use die() for error handling. Provide a properly formatted HTML page to the user. There is no reason to use die() except in some extreme cases

 

2. Validation of the data should be done when saving to the database. E.g. if a value should be numeric, a date, email, etc. Only save the data if it passes validation. The data should be appropriately processed to avoid SQL injection (or other types of errors based on how you are using it): prepared statements or a proper escape function. You should not escape the data for HTML purposes (or another intended output- see #3).

 

3. When presenting data (such as in an HTML page) you should use the appropriate functions/processes to escape the data for the intended output. You do not want to escape for the output when storing it. You never know what output formats you may need. By leaving the data in it's original state you don't have to unescape from one format and then escape to another. For example if you were to escape for HTML output before saving and then you needed to repurpose the data for a JSON call.

[ajoo]

Hi Psycho, 

 

Thanks for the advice. 

 

I won't really use die, but then can't this be considered as an extreme case if I find that data stored in the database, which was stored after data validation, has a different data type than was initially inserted, on retrieval from the DB. However I intend to use the 

 

 

http_response_code(400);

 

I think I am doing exactly as you have mentioned in 2 and 3. i.e. I am doing the validation before saving the data to the database if they pass the criteria. I do not sanitize any inputs or change them in any way. I escape the output only while displaying the data from the DB. I am however validating the data from the DB upon retrieval. I thought it was a bit cumbersome but I am doing it anyways.

 

Thanks loads !

[Phi11W]

... I do recall reading somewhere, data should never be trusted even if it is coming from a DB. I believe there are cases when the data of a particular user may be compromised and as against the entire DB. In such cases, these checks will restrict attacks, that would otherwise occur, like stored xss attacks as mentioned by stratadox.

 

You should be able to trust the data in your database.  If that's wrong, then you're in a very Bad Place.

You should try to protect "your" data at all times and that means checking, validating and, where necessary, encoding data on its way in; thereafter, it can be trusted. 

 

It's one of the oldest computing adages there is - "Garbage In; garbage out". 

 

These checks will not protect your site.  All they will accomplish is to crash your web site after an attack has happened.  If the data's been compromised, your checks will fail and nobody will get to see anything (except error messages) ...   and the phones on the Help Desk will start ringing off the hook.  

 

Regards,   Phill  W.

[ginerjm]

I'm curious how you can do a fetch directly into a set of vars instead of an array. Are you using mysqlI bind functions to do this?

 

Stratadox: I'm not familiar with the "html_escape" function. Couldn't find it in the manual.

[Barand]

I'm not familiar with the "html_escape" function. Couldn't find it in the manual.

You won't find it in the manual but you will find it half-way down reply #5 of this thread.

[Stratadox]

I have extensively checked fcheckEmail and html_escape function. html_escape was provided by Guru Jacques, so can't be wrong. Here's the code for these below for you to verify.

 

 

function fcheckEmail($str) // to validate the format of an email.
{
$str = filter_var($str, FILTER_VALIDATE_EMAIL);
return $str;} 

I understand the issues with filter_var but my intent here is to check that the data passed by the user is in the valid format. 

function html_escape($raw_input)
{
    return htmlspecialchars($raw_input, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
}

 

I think the above stems from the fact that you are assuming that the functions do not do what they say they do. So I have put in the snippet functions for you to see and check if you would like to.

 

The functions seem to do exactly what I expected they would do.

Thank you for posting the function, I can now explain in more detail why your checks do not work.

 

Because no, they don't

 

The functions themselves are correct, but the way you're using them is not.

 

Let's take the following example:

 

if(($email = html_escape(fcheckEmail($email)))=== false)

The following steps will be taken if the input is not valid:

- fcheckEmail will be executed with $email as parameter and return false

- html_escape will be executed with false as parameter and return ''

- $email will be set to ''

- the if condition checks if '' is exactly the same as false

- it is not and the program continues as if the email was valid

 

As you see, the die message will never be shown, even for invalid data.

 

 

 

3) Attacks on your users, for example stored xss attacks

Yes, That's what I am seeking to avoid by these checks.

 

Except you don't need these checks for that purpose, all you need is:

3) Escaping manipulatable data

[ajoo]

Hi all !!

 

Sorry for the delayed response as I was away for a few days. 

 

@ginerjm : 

 

 

I'm curious how you can do a fetch directly into a set of vars instead of an array. Are you using mysqlI bind functions to do this?

 

Yes that's correct.

bind_result($lname, $ fname, $date, ...);

 

@stratadox :

 

 

 

Because no, they don't 

 

Correct ! Thanks for that. You are right. I have only recently, since my last 2 posts, added the html_escape to the fcheck functions missing out on some good advise by Guru Jacques of using the html_escape function only while outputting the data. My fault. Thanks a lot for pointing out this blunder. 

 

@ Guru Barand : It would be great to hear your views on this. I mean on the idea of checking / validating the data fetched from the DB. Should it be done or not as every one else seems to think that it's redundant. 

 

@ Phi11W: Hi, Thanks for reply. 

 

 

 

encoding data on its way in

May I request you to elaborate on this.   

 

 

 

 If the data's been compromised, your checks will fail and nobody will get to see anything (except error messages) ...   and the phones on the Help Desk will start ringing off the hook.  

 

 

Irrespective of whether I check the value of the data from the DB or not, wouldn't this happen anyway if a site is hacked and data compromised? 

 

 

Thanks all !