Save user input in server-side database

Question

I have created a site for practice and I have already asked a question on Stack Overflow about XSS.

Someone tell me to show your php functions on this site that i created using filter_Var

These are two PHP functions I created that sanitize/validate user input and after passing the the user input in these function I save the user input in my server (storage).

function Meg_selfvalidator_html($data){//Validator
 if(is_array ($data)){
 $arrayLen = count($data);
 for($i=0;$i<$arrayLen;$i++){
 $data[$i] = trim($data[$i]);
 $data[$i] = stripslashes($data[$i]);
 $data[$i] = htmlentities($data[$i], ENT_QUOTES);
 $data[$i] = filter_var($data[$i], FILTER_SANITIZE_STRIPPED, FILTER_FLAG_STRIP_HIGH );
 $data[$i] = filter_var($data[$i], FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_HIGH);
 $data[$i] = filter_var($data[$i], FILTER_SANITIZE_STRIPPED, FILTER_SANITIZE_STRING);
 }
 return $data;
 }else{
 $data= trim($data);
 $data= htmlentities($data, ENT_QUOTES);
 $data = stripslashes($data);
 $secured_output = filter_var($data, FILTER_SANITIZE_STRIPPED, FILTER_FLAG_STRIP_HIGH );
 $secured_output = filter_var($secured_output, FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_HIGH );
 $secured_output = filter_var($secured_output, FILTER_SANITIZE_STRIPPED, FILTER_SANITIZE_STRING);
 return $secured_output;
 }
}

This is my second PHP function:

function selfValidator($data) {//Validator
 if(is_array ($data)){
 $arrayLen = count($data);
 for($i=0;$i<$arrayLen;$i++){
 $data[$i] = trim($data[$i]);
 $data[$i] = stripslashes($data[$i]);
 $data[$i] = strip_tags($data[$i]);
 $data[$i] = htmlspecialchars($data[$i]);
 $data[$i] = filter_var($data[$i], FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_HIGH);
 }
 return $data;
 }else{
 $data = trim($data);
 $data = stripslashes($data);
 $data = strip_tags($data);
 $data = htmlspecialchars($data);
 return $data;
 }
}

Which one is the best function or what is bad in these functions? How do I add a filter to validate the link?

• BCdotWEB: "How do I add a filter to validate the link?" is off-topic for this site.

Answer 1

Neither is all that good. It seems that you just apply functions at random to your data in the hope that some of them will protect you, which is not the correct approach to security.

How to properly defend against XSS

First of all, the correct approach to XSS is to HTML encode data when outputting it. That is where the vulnerability is create, and that is where it should be prevented.

There are three important reasons for this:

1. When outputting, you know the context, which is important to defend against XSS (is it inside script tags? inside a HTML attribute? What attribute? A JavaScript context attribute? Are there quotes around it? What quotes? ...)
2. You really can't be sure when outputting data that it was previously secured (can data get into the database in other ways than GET/POST? etc).
3. Your data will not be clean anymore, which may break your website (eg when sending an email, or when using passwords (with your function, "a<super!secure^password" would become a)).

Note that there are contexts in which HTML encoding may not be enough to prevent XSS.

Improving your Code

Why are you using different cleaning functions for array values and non-array values in selfValidator? It doesn't make any sense to treat these differently.

For that matter, if you would use the same functions, you would have duplication in your code, which should be removed (either by using recursion or by extracting the cleaning to a separate function).

Also:

• Use more spaces, eg around <, =, etc (just use any IDE to fix formatting issues for you).
• Your variable names are not consistent. Sometimes you use secured_output and sometimes data. When things are named differently, you make a reader of the code wonder why that is (eg is data maybe not secured)?
• Your Validator comment doesn't really add any information, I would just remove it (and give the functions a more fitting name instead). You could create PHPDoc style comments instead, documenting the behavior of your function in-depth

The cleaning functions you use

I think it would be a good idea to look at the functions you do use in-depth, to get a real understanding what each of them does. Ideally, you take a look at the PHP documentation for this, but here is a short overview:

• trim: remove whitespace, may be useful, depending on the situation
• stripslashes: ONLY use this when magic quotes are enabled. Stripping slashes does nothing for security, and it makes your data unclean if magic quotes are disabled (which is the default).
• htmlentities: This is what you generally want to use to prevent XSS when outputting data in most contexts.
• strip_tags: Not really all that useful. Doesn't prevent XSS in as many contexts as htmlentities, and makes your data dirtier than htmlentities (see my password example above).
• filter_var: These functions are great when first accessing GET/POST values, to restrict what characters are allowed (as defense in depth, NOT as only defense). If you know that you need only alphanum, it's good to restrict the characters, etc (just in case that you do have vulnerabilities in other parts of your application, this will make it harder to exploit them). But: You can't just apply filter_var to all values that you will ever receive. It's again context dependent. If you expect an id, only accept integer. If you expect an email address, use that filter. But if you eg expect a password, filtering it is a bad idea, as it reduces the complexity of the password.

• Aryan: so should i use only filter_var () ?? i am using array and non array because i am using this function in quite all files so if i pass an array it will check for array values and if i pass variable so it will check for only variable. wouldn't be better to use htmlspecialchars()
• tim: @Aryan I understand why you process arrays and single values. But you do different things with them, which doesn't make sense. Regarding the rest: I'm not saying that you should use function X or function Y exclusively. What I'm saying is that you should use the correct function at the correct time, and have a proper security concept.
• tim: @Aryan Just having one generic filter that filters all input is not a very good concept. You should filter the input according to what input it is. So if you expect an id, only accept integers; If you expect some alphanum string, only accept that, etc. BUT: this input filter is only an additional safety mechanism. It is NOT your main line of defense. Against XSS, that would mainly - but not only, depending on context - be HTML encoding, eg with htmlspecialchars when outputting data (ideally, this would happen by default).