CodeIgniter Forums
A little confusion about security - Printable Version

+- CodeIgniter Forums (https://forum.codeigniter.com)
+-- Forum: Using CodeIgniter (https://forum.codeigniter.com/forum-5.html)
+--- Forum: Best Practices (https://forum.codeigniter.com/forum-12.html)
+--- Thread: A little confusion about security (/thread-62900.html)



A little confusion about security - Urastor - 09-06-2015

Hey there,

I rewrote some parts of my admin-system, no I got confused.

I'm using the form-helper to generate forms and the set_value(); function to set data if exists or if it's in the database. In the past I used this:

PHP Code:
$this->form_creator->setValue('example_field', (set_value('example_field') == false) ? $entry->value set_value('example_field')); 

(Ignore the form_creator thingy)
As you can see, if set_value can't find post-data, it uses the Default value from the db. After reading the docs a Little bit, I found out that set_value actually does exactly the same, so I decided to use this instead:

PHP Code:
$this->form_creator->setValue('example_field'set_value('example_field'$entry->value)); 

The Problem: The textfield needs to contain html-tags, like a <p> or <blockquote>. I live in Germany, so I use the ASCII_to_entities method, to convert characters like ä, ü, ö, ß, double quotes, single quotes, and some others. Because we get our articles from Microsoft Office and other Office programs, which uses those characters, they're automatically converted to, i.e ä => ä, which is absolutly great, because this means, we don't Need to care much.
Now, if I load the form, because of set_value, those characters are converted to, i.e. &amp;#228;, when I save this, everything goes wrong. Next time I edit the same form, it converts $amp;#288; to &amp;amp;#288;.

I decided to take the third paramater, and turn off html-escaping.

My question: In what way is this save? Could any Code be executed or harming, if I use the third Parameter? Of Course, JavaScript can be executed by this, so it's not very xss-friendly..

I could use a Parser for the tags, but I Need to use ASCII_to_entities for the Office characters etc., which is a Problem..

Any suggestions, tips or?

Greetings, Uri


RE: A little confusion about security - Diederik - 09-07-2015

I hope I understand your question correctly, I think you could use the xss_clean() for this job. It will allow you to populate the inputs in your form with the (unescaped) posted data and then have xss_clean() filter the entire output before sending it back to the browser.


PHP Code:
// insert your form validation code

if ($this->form_validation->run() == FALSE) {
    
    
// Form either not valid or no post at all
    $form  $this->load->view('form'$viewdatatrue);

    if ($this->input->server('REQUEST_METHOD') == 'POST') {
        // Only use XSS clean if there actualy is some POST data to filter
        $filtered $this->security->xss_clean($form); 
        echo $filtered;
    } else {
        echo $form;
    }
} else {

    // form input was succesfully validated





RE: A little confusion about security - Urastor - 09-07-2015

(09-07-2015, 04:45 AM)Diederik Wrote: I hope I understand your question correctly, I think you could use the xss_clean() for this job. It will allow you to populate the inputs in your form with the (unescaped) posted data and then have xss_clean() filter the entire output before sending it back to the browser.

Hello Diederik,

First, thanks for your reply. Smile

Because I use CI's input library, my post data is automatically escaped. The problem is, before I send form data to my data base, I use ascii_to_entities($this->input->post('content')) to ensure, all characters like ", ', ä, etc. are converted to they're entity, for example an apostrophe in word has the code 145 (I think), but I need the code 39, to make it actually appear in my browser. ascii_to_entities() does this. It converts the apostrophe to ', which is the apostrophe I want.
However, if I use the form_helper, which internally uses html_escape, it converts & to &amp;. The next time I save this, ascii_to_entities() converts & and the whole thing repeats.

Because I don't use bb-code to create my post-entries, the html_escape()-function converts tags like <p>, <br> and so on, to i.e. &lt;br&gt;, which is absolutly ok, because it prevents executing for example javascript (<script>alert(document.cookie)</script>)

When i turn the html_escape of (by using i.e. form_input('field', 'value', false)Wink, it is a security issue, because the js-code on top could be executed.

Hope this was more clear, don't know how to describe this kind of problem.. Confused




RE: A little confusion about security - mwhitney - 09-08-2015

Does your database have problems storing the characters correctly (and, if so, is there a reason why you can't fix it, for example by changing the character set of the database)?

The input library does not automatically escape your post data unless you turned on global XSS filtering, which is not recommended.

Functions like ascii_to_entities() should be used (if at all) when outputting to HTML, not when processing data for the database, unless the database uses a character set which doesn't support the characters and the database cannot be changed.

There is no parameter for the form_input() function in the form_helper which will disable the use of html_escape() on the value. So, you can pass the third parameter to set_value() to disable its use of html_escape() when you're passing the result of set_value() to form_input()'s second parameter, but form_input() will always use html_escape() on the value.

One thing which can be done is to use:
PHP Code:
<input name="input_name" value="<?php echo html_escape(set_value('field', 'value', false), false); ?>" /> 

If you really want to encode everything in your input, you could use:
PHP Code:
<input name="input_name" value="<?php echo htmlentities(set_value('field', 'value', false), ENT_QUOTES, config_item('charset'), false); ?>" /> 

(or you could add a suitable helper function to pass the second and third parameters to htmlentities() as html_escape() does with htmlspecialchars(), especially since you may want to pass some additional flags to the second parameter to indicate the target format, such as ENT_HTML5 or ENT_HTML401).

In html_escape(), htmlspecialchars(), and htmlentities(), the last parameter can be set to false to attempt to prevent double-encoding the data, basically checking whether & is the start of an entity before converting it, so you don't end up with &amp;amp; or &amp;lt;.


RE: A little confusion about security - Martin7483 - 09-18-2015

When saving data to the DB I use the following method:

Code:
public function strToDB($string) {
   if(is_array($string)) {
       foreach($string as $key => $value) {
           $string[$key] = $this->clean_string($value);
       }
       return $string;
   } else {
       $string = htmlspecialchars($string, ENT_QUOTES, "UTF-8");
       $string = mysql_real_escape_string($string);
       $string = str_replace('\r\n', PHP_EOL, $string);
       $string = str_replace('\n', PHP_EOL, $string);
       return $this->xss_clean($string);
   }
}
This method will convert special chars for saving in your DB

When retrieving data from the DB I use the following method
Code:
public function strFromDB($str) {
    if( is_string($str) ) {
        $str = trim(html_entity_decode($str, ENT_QUOTES, "UTF-8"));
        $str = stripslashes($str);
        return $str;

    } else {
        return $str;
    }
}

The method will reverse what the first method did, so that you can edit the data or print to screen.

Hope this can help you


RE: A little confusion about security - mwhitney - 09-18-2015

(09-18-2015, 05:55 AM)Martin7483 Wrote: When saving data to the DB I use the following method:


Code:
public function strToDB($string) {
   if(is_array($string)) {
       foreach($string as $key => $value) {
           $string[$key] = $this->clean_string($value);
       }
       return $string;
   } else {
       $string = htmlspecialchars($string, ENT_QUOTES, "UTF-8");
       $string = mysql_real_escape_string($string);
       $string = str_replace('\r\n', PHP_EOL, $string);
       $string = str_replace('\n', PHP_EOL, $string);
       return $this->xss_clean($string);
   }
}
This method will convert special chars for saving in your DB

In most cases, you should use $this->db->escape() rather than mysql_real_escape_string(). $this->db->escape() will use the function appropriate for the currently configured database and will be more likely to continue to work in PHP 7, since the mysql_* functions were deprecated in 5.5 and removed in 7. Additionally, escaping the data should be the last thing done before putting the data into the database, which is why many of the database functions do it for you.

Why are you using htmlspecialchars() on data to be stored in a database? The whole purpose of this function is to encode data to be output as HTML. Plus the conversions performed by htmlspecialchars() are limited to a handful of characters, and, by setting ENT_QUOTES, you've ensured that at least two of the characters handled by mysql_real_escape_string() will no longer be in your strings (and nearly half of those which are left you're going to go on to mess with after you call mysql_real_escape_string(), though I don't know how you expect to find a '\r\n' after you've replaced them all with '\\r\\n').

Further, if you use CI's html_escape() instead of htmlspecialchars(), it would at least use config_item('charset') as the third parameter to ensure the encoding matches what's used in the rest of the system (as should be done on html_entity_decode() as well), though this still wouldn't be the appropriate place to use the function.

Finally, to quote the docs for the Input class:
Quote:XSS escaping should be performed on output, not input!

Perhaps that should be even more specific, in that it should be performed on output to HTML.

Quote:When retrieving data from the DB I use the following method

Code:
public function strFromDB($str) {
    if( is_string($str) ) {
        $str = trim(html_entity_decode($str, ENT_QUOTES, "UTF-8"));
        $str = stripslashes($str);
        return $str;

    } else {
        return $str;
    }
}

The method will reverse what the first method did, so that you can edit the data or print to screen.

No, this does not reverse what the first method did. html_entity_decode() does the opposite of htmlentities(), which could be quite a bit more than just reversing html_special_chars(). Further, between this and stripslashes, you've potentially undone some of the work done by xss_clean(), and this is the point at which it might be appropriate to actually use xss_clean(). (For example, xss_clean() will replace '<?' with '&lt;?' and '?>' with '?&gt;', and html_entity_decode() will convert them back.)


RE: A little confusion about security - Martin7483 - 09-22-2015

Thanks mwhitney,

I have been using this for years now, and never had any problems. Time to update!
Do you have any good examples?


RE: A little confusion about security - mwhitney - 09-22-2015

As I stated before, most of the db functions (especially in Query Builder) will escape the data before it's placed in the database, but you can use $this->db->escape() directly when needed. The key point is to prepare the data for wherever you are currently putting it, not for where you think it's going to end up later.

When storing data in the database, focus on what is required to safely store it in the database: validate the data to make sure it is of the right type and within the range to safely store it in the database fields, then escape it and/or use bound queries to help avoid SQL-injection attacks. If your database's encoding doesn't match your form's encoding, you may need to manipulate the data accordingly, but this is the domain of functions like mb_convert_encoding() (before escaping the data/binding it to your queries). If you start using functions which deal with HTML, JavaScript/JSON, etc., then you're not focused on the current task, because somewhere down the line you may find a reason to output the same data in multiple formats, and you shouldn't assume that data in your database is safe for output in any given format.

When sending data to a view, focus on what is required to safely display data in the current view. If your database encoding doesn't match your output encoding, you'll need to convert it again, but even at this point you should be using functions specific to that task, rather than doing it as a side-effect of HTML entity encoding or some similar process.

If the view is HTML, and you're going to manipulate any HTML entities in the data, you're going to want to use htmlentities() or htmlspecialchars() at this point (after the encoding conversion, if necessary), as the browser can easily decode them where appropriate. If you want to normalize the EOL characters, this might be the appropriate time, though the EOL characters in your data are probably never going to matter if your output format is HTML, except maybe as a very inefficient form of compression which could be better handled elsewhere. The last thing you should do before output is use a function like xss_clean() on any character/string or text. The point of doing this last is that any of the previous operations could potentially undo the work done by a function like xss_clean() to protect your output (in cases where it doesn't remove something completely).

If the view (or target output format) is JSON, or some other format, chances are that you will need a completely different set of functions to verify that the data is safe for output.