Best practice of handling user's input

Hi,

I wonder what is the best and the most secured way of handling user's input. Basically I have form for user's profile made by form helper like this: 

Code:

echo form_open();
   echo form_label($this->lang->line('user_update_profile_first_name'), 'first_name');
   echo form_input(array('type' => 'text', 'name' => 'first_name', 'id' => 'first_name', 'maxlength' => '255', 'required' => 'true', 'value' => set_value('first_name', $user_profile['first_name'], false)));

   echo form_label($this->lang->line('user_update_profile_last_name'), 'last_name');
   echo form_input(array('type' => 'text', 'name' => 'last_name', 'id' => 'last_name', 'maxlength' => '255', 'required' => 'true', 'value' => set_value('last_name', $user_profile['last_name'], false)));

   echo form_label($this->lang->line('user_update_profile_birth_date'), 'birth_date');
   echo form_input(array('type' => 'text', 'name' => 'birth_date', 'id' => 'birth_date', 'maxlength' => '255', 'required' => 'true', 'value' => set_value('birth_date', $user_profile['birth_date'],

   echo form_submit(array('value' => $this->lang->line('user_update_profile_form_submit'), 'name' => 'submit', 'class' => 'btn btn-primary'));
echo form_close();

As you can see in my code I am skipping xss filtering provided in set_value function due to xss filtering is done in form_input() already. My Controller function for inserting data in DB looks like this:

Code:

$validation_rules = array(
   array(
       'field' => 'first_name',
       'label' => $this->lang->line('user_update_profile_validation_error_first_name'),
       'rules' => 'required|trim|max_length[255]'
   ),
   array(
       'field' => 'last_name',
       'label' => $this->lang->line('user_update_profile_validation_error_last_name'),
       'rules' => 'required|trim|max_length[255]'
   ),
   array(
       'field' => 'birth_date',
       'label' => $this->lang->line('user_update_profile_validation_error_birth_date'),
       'rules' => 'required|trim|max_length[255]'
   )
);

$this->form_validation->set_rules($validation_rules);
if($this->form_validation->run()) {
   $user_data = array(
       'user_id' => $this->profile_data->user_id,
       'first_name' => $this->input->post('first_name', TRUE),
       'last_name' => $this->input->post('last_name', TRUE),
       'birth_date' => date('Y-m-d',strtotime($this->input->post('birth_date', TRUE)))
   );

   if($this->user_model->update_user_profile($user_data)) {
       $view_data['success'] = TRUE;
       $new_site_language = $this->language_model->getLanguageFolderById($user_data['site_language']);
       $this->lang->load('application/user_lang', $new_site_language);

   } else {
       $view_data['server_error'] = TRUE;
   }
}

I am filtering here data from user by provided $this->input->post('', true) xss filter. In model I am inserting data to DB by active record class. I am just wondering if this is the right and secure way of handling users input if there is not needed something like htmlspecialchars() . But what happens when someone have some "special" chars in name like for example Someone O'Sombody or some names from foreign countries? I am also showing data in navbar using html_escape($this->profile_data->first_name) to prevent running users potentially dangerous code. Did I get this whole "security thing" in the right way or there should be something changed because of potential danger?

You're mixing input handling with concerns over output-based vulnerabilities.

Input should be validated, not filtered; i.e. don't use $this->input->post('var', true) before saving to a database. What goes into a database should only be guarded against SQL injections.
Apply XSS filtering (xss_clean()) only when printing the data to a browser.

Also, your mention of "active record" implies that you're still using CodeIgniter 2.x (because it's no longer called AR in CI3), which is no longer supported. The easiest thing to get right about security is always keeping software up to date - using unsupported versions means that you're not getting potential security patches.

Hi,

I wonder what is the best and the most secured way of handling user's input. Basically I have form for user's profile made by form helper like this: 

Code:

echo form_open();
   echo form_label($this->lang->line('user_update_profile_first_name'), 'first_name');
   echo form_input(array('type' => 'text', 'name' => 'first_name', 'id' => 'first_name', 'maxlength' => '255', 'required' => 'true', 'value' => set_value('first_name', $user_profile['first_name'], false)));

   echo form_label($this->lang->line('user_update_profile_last_name'), 'last_name');
   echo form_input(array('type' => 'text', 'name' => 'last_name', 'id' => 'last_name', 'maxlength' => '255', 'required' => 'true', 'value' => set_value('last_name', $user_profile['last_name'], false)));

   echo form_label($this->lang->line('user_update_profile_birth_date'), 'birth_date');
   echo form_input(array('type' => 'text', 'name' => 'birth_date', 'id' => 'birth_date', 'maxlength' => '255', 'required' => 'true', 'value' => set_value('birth_date', $user_profile['birth_date'],

   echo form_submit(array('value' => $this->lang->line('user_update_profile_form_submit'), 'name' => 'submit', 'class' => 'btn btn-primary'));
echo form_close();

As you can see in my code I am skipping xss filtering provided in set_value function due to xss filtering is done in form_input() already. My Controller function for inserting data in DB looks like this:

Code:

$validation_rules = array(
   array(
       'field' => 'first_name',
       'label' => $this->lang->line('user_update_profile_validation_error_first_name'),
       'rules' => 'required|trim|max_length[255]'
   ),
   array(
       'field' => 'last_name',
       'label' => $this->lang->line('user_update_profile_validation_error_last_name'),
       'rules' => 'required|trim|max_length[255]'
   ),
   array(
       'field' => 'birth_date',
       'label' => $this->lang->line('user_update_profile_validation_error_birth_date'),
       'rules' => 'required|trim|max_length[255]'
   )
);

$this->form_validation->set_rules($validation_rules);
if($this->form_validation->run()) {
   $user_data = array(
       'user_id' => $this->profile_data->user_id,
       'first_name' => $this->input->post('first_name', TRUE),
       'last_name' => $this->input->post('last_name', TRUE),
       'birth_date' => date('Y-m-d',strtotime($this->input->post('birth_date', TRUE)))
   );

   if($this->user_model->update_user_profile($user_data)) {
       $view_data['success'] = TRUE;
       $new_site_language = $this->language_model->getLanguageFolderById($user_data['site_language']);
       $this->lang->load('application/user_lang', $new_site_language);

   } else {
       $view_data['server_error'] = TRUE;
   }
}

I am filtering here data from user by provided $this->input->post('', true) xss filter. In model I am inserting data to DB by active record class. I am just wondering if this is the right and secure way of handling users input if there is not needed something like htmlspecialchars() . But what happens when someone have some "special" chars in name like for example Someone O'Sombody or some names from foreign countries? I am also showing data in navbar using html_escape($this->profile_data->first_name) to prevent running users potentially dangerous code. Did I get this whole "security thing" in the right way or there should be something changed because of potential danger?

I prefer simple stuff, security = know what the fuck are you doing and what are you receiving from the user, so I do same than you do except than I build my own forms with simple html and... I filter the data I receive the next way:

Code:

//Example
$this->form_validation->set_rules('entity_id', 'entity_id', 'trim|xss_clean|prep_for_form|htmlspecialchars|is_natural');
$this->form_validation->set_rules('street', 'Calle', 'trim|required|xss_clean|prep_for_form|htmlspecialchars|min_length[2]|max_length[500]');

• trim: to be sure than the info have no tabs, nulls or spaces
  
• xss_clean: remove wierd js injections, php tags, non printable characters and a lot of kind of attacks xss (check core/Security.php )
  
• prep_for_form: avoid tags/quotes break my form
  
• htmlspecialchars: final hit to be complete secure than nothing is bypassing the validations
  

then common validations... max_length, is_natural, etc

I hope my opinion help you.

You're mixing input handling with concerns over output-based vulnerabilities.

Input should be validated, not filtered; i.e. don't use $this->input->post('var', true) before saving to a database. What goes into a database should only be guarded against SQL injections.
Apply XSS filtering (xss_clean()) only when printing the data to a browser.

Also, your mention of "active record" implies that you're still using CodeIgniter 2.x (because it's no longer called AR in CI3), which is no longer supported. The easiest thing to get right about security is always keeping software up to date - using unsupported versions means that you're not getting potential security patches.

I am using CI3 but I was just used to call it active record I meant query builder sorry for that. Here is my model code.

Code:

   public function update_user_profile($user_data) {
       $this->db->where('user_id', $user_data['user_id'])
                ->set($user_data)
                ->update(config_item('customer_profiles'));

       if( $this->db->affected_rows() == 1 ) {
           return TRUE;
       }
       return FALSE;
       
   }

This is automatically checked against SQL injections isn't it? So you are saying that I should not be filtering data on input to DB? Is there any reason for that?

Let's assume that I am filtering data on output not input, so it's possible that I will have in my DB as user's first name something like this?

Code:

You are hacked: <script>alert('Lorem Ipsum');</script>

Is that good way of storing data? And what is the best way of showing it when I want to have displayed user's first name somewhere?

Code:

html_escape();

Or

Code:

$this->security->xss_clean();

Combination of both?

I prefer simple stuff, security = know what the fuck are you doing and what are you receiving from the user, so I do same than you do except than I build my own forms with simple html and... I filter the data I receive the next way:

Code:

//Example
$this->form_validation->set_rules('entity_id', 'entity_id', 'trim|xss_clean|prep_for_form|htmlspecialchars|is_natural');
$this->form_validation->set_rules('street', 'Calle', 'trim|required|xss_clean|prep_for_form|htmlspecialchars|min_length[2]|max_length[500]');

• trim: to be sure than the info have no tabs, nulls or spaces
  
• xss_clean: remove wierd js injections, php tags, non printable characters and a lot of kind of attacks xss (check core/Security.php )
  
• prep_for_form: avoid tags/quotes break my form
  
• htmlspecialchars: final hit to be complete secure than nothing is bypassing the validations
  

This shows that you don't know what you're talking about.

Code:

   public function update_user_profile($user_data) {
       $this->db->where('user_id', $user_data['user_id'])
                ->set($user_data)
                ->update(config_item('customer_profiles'));

       if( $this->db->affected_rows() == 1 ) {
           return TRUE;
       }
       return FALSE;
       
   }

This is automatically checked against SQL injections isn't it?

It automatically escapes the data, yes. But you should always validate it regardless of that and here you aren't even checking what fields are there in $user_data.

So you are saying that I should not be filtering data on input to DB? Is there any reason for that?

Yes and yes.

I've been asked this many times and I can't explain it in detail every time - sorry. I know it sounds logical to you that if you automatically escape every input, it should be completely safe afterwards, but that's not how it works.

Security is hard and it's a professional field of its own ... don't try to be smart with it because 99% of the time that just introduces more flaws. Just handle the details with precision - if it's an attack made possible on output, then handle it when outputting.

Let's assume that I am filtering data on output not input, so it's possible that I will have in my DB as user's first name something like this?

Code:

You are hacked: <script>alert('Lorem Ipsum');</script>

Is that good way of storing data? And what is the best way of showing it when I want to have displayed user's first name somewhere?

Code:

html_escape();

Or

Code:

$this->security->xss_clean();

Combination of both?

If you validate your inputs, you'll never have a user's first name like that - you know that people's names don't include parentheses, slashes and greater-than/less-than signs, so don't accept these.

Take note of what "validation" means - the former means checking if the data is valid and completely rejecting it if it's not. This is what you should be doing.

That being said, a free text description could look like that and yes - that's how it should be saved into the database. xss_clean() is enough to make it safe for output.
html_escape() is OK too, unless you'd be putting that value inside JavaScript code or an HTML attribute. It just has a more limited scope.

Again, don't do both - you can only corrupt valid data this way, with no real benefits.

It automatically escapes the data, yes. But you should always validate it regardless of that and here you aren't even checking what fields are there in $user_data.

This is done in controller in validation or that's not enough? Is here needed some more specific sort of validation?

If you validate your inputs, you'll never have a user's first name like that - you know that people's names don't include parentheses, slashes and greater-than/less-than signs, so don't accept these.

Take note of what "validation" means - the former means checking if the data is valid and completely rejecting it if it's not. This is what you should be doing.

This is kinda a tricky in this case, for example I am not aware of all possible characters that are used in names across the countries. Different countries means different names and maybe some signs like ' etc. This means that I should write some own validation rule for CI because there is nothing like that in validation class am I right?

It automatically escapes the data, yes. But you should always validate it regardless of that and here you aren't even checking what fields are there in $user_data.

This is done in controller in validation or that's not enough? Is here needed some more specific sort of validation?

It's enough, I was only looking at the model when I wrote this.

If you validate your inputs, you'll never have a user's first name like that - you know that people's names don't include parentheses, slashes and greater-than/less-than signs, so don't accept these.

Take note of what "validation" means - the former means checking if the data is valid and completely rejecting it if it's not. This is what you should be doing.

This is kinda a tricky in this case, for example I am not aware of all possible characters that are used in names across the countries. Different countries means different names and maybe some signs like ' etc. This means that I should write some own validation rule for CI because there is nothing like that in validation class am I right?

Yep.

It automatically escapes the data, yes. But you should always validate it regardless of that and here you aren't even checking what fields are there in $user_data.

This is done in controller in validation or that's not enough? Is here needed some more specific sort of validation?

It's enough, I was only looking at the model when I wrote this.

If you validate your inputs, you'll never have a user's first name like that - you know that people's names don't include parentheses, slashes and greater-than/less-than signs, so don't accept these.

Take note of what "validation" means - the former means checking if the data is valid and completely rejecting it if it's not. This is what you should be doing.

This is kinda a tricky in this case, for example I am not aware of all possible characters that are used in names across the countries. Different countries means different names and maybe some signs like ' etc. This means that I should write some own validation rule for CI because there is nothing like that in validation class am I right?

Yep.

Great thanks a lot, I already used regex that I found somewhere to check if it contains "bad" chars.