• 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Safe escape

#1
Hello,
When I do something like this:
Code:
$data = array(
     'user_id' => $id,
     'followed_user_id' => $followed,

);
$this->db->insert('users_follows', $data);




It's safe and CI will automatically escape the variables, right?

But when I do smth like this:

Code:
$this->db->join('cities as s', 's.state_id = "'.$GET['state'].'" AND s.id = users.city_id');
or

Code:
$data = array(
   'img' => $src,
   'img_s' => $src2,
   'img_xs' => $src3
);
$this->db->update('users', $data, "id = '".$id."'");  

Will CI also escape the variables?

Thanks !
Reply

#2
It will try to, but don't do that. Escaping or not, always validate user input - that should be your number one rule.
Reply

#3
Of course I always validate user input, thanks. So my queries are ok or I should change something?
Reply

#4
You wouldn't be asking that if you validated the input.
Reply

#5
Your code is bad practice for security.
Validation has nothing to do with it.
Do you want to write tests for your apps? Our book, CodeIgniter Testing Guide would help you.
Reply

#6
(08-12-2015, 09:06 PM)kenjis Wrote: Your code is bad practice for security.
Validation has nothing to do with it.

So, could you show me good practice?
Reply

#7
Don't concatenate variables to build SQL statement.
Do you want to write tests for your apps? Our book, CodeIgniter Testing Guide would help you.
Reply

#8
Safe practice: filter(/validate) input, escape output.

Generally, I pass $this->input->post() through the form validation library, using a combination of rules and functions I've defined in my model to only grab the specific fields that are going to be used by my model and make sure they match the data types and ranges valid for my database. All of the valid data goes into a new variable (usually an array called $data), and, ideally, I never touch $this->input->post() again (and never touch $_POST at all, if I can help it). I try to avoid using $_GET or $this->input->get(), but the same theory applies as with $_POST and $this->input->post().

Many times I'll apply custom validation rules to some (or all) of my data, including using functions which apply PHP's built-in filters, and anything else I can think of that would be important for ensuring the data is valid.

All of this is the "filter input" phase. The next part, of course, is "escape output", but many people assume that we're talking about things like htmlspecialchars(). That's not the case, because we're not outputting to HTML, yet. We're outputting to a database. When building your SQL strings, make sure you're still using the filtered/validated data ($data), and not falling back to $this->input->post()/get() or $_POST/$_GET. I think this is the point that caused a lot of red flags to go off for people seeing the following code in your post:

Code:
$this->db->join('cities as s', 's.state_id = "'.$GET['state'].'" AND s.id = users.city_id');

It's very possible that $GET[] could be your version of $data, but it's really hard to tell whether that's the case or it was simply a typo of $_GET. Further, I find that the join itself poses a lot of questions in my mind about the database structure and the reasoning for building a join on user input.

To answer your question, query builder will TRY to escape whatever you pass to it (unless you tell it not to). In most cases, it will do the best it can to escape your data so that it will safely go into your database. If it fails to escape something properly, you will be more likely to get a database error than to have some sort of SQL injection, but I'm sure it's still possible at some level if the data is really well-crafted and poorly filtered.

Remember, though, that I said "escape your data so that it will safely go into your database". That's really all that query builder should be concerned with, and you should remember that this makes the data in your database safe for no purpose other than existing in your database.


Anyway, to continue, once you turn around and display the data, you "filter input, escape output" all over again. In this stage, the input is coming from the database and the output is whatever form you've decided to use for display (whether it's HTML for a view, JSON output to send to JavaScript, generating a PDF or other file format, or even creating a GET/POST request for some other website's API). While you wouldn't use the form_validation library to filter your input, and in most cases you can be much more accepting of the input from your database than the input from a user, you still should not consider the data to be any safer at this stage than at any other. For all you know, the database was compromised or something dangerous got by your attempts to filter the input before it was inserted into the database. If you're outputting HTML, this is where xss_clean() and htmlspecialchars() are your friends.

The web is a wide-open resource for information about doing these things safely, but there is, unfortunately, as much bad information as good. If you're reading advice that violates the "filter input, escape output" principle, then it's probably not good (as long as you do understand that this is input to your application and output from your application, that your database, your JavaScript, your HTML are all potentially part of the input or the output, and never part of your application).

OWASP has a lot of good, general-purpose information about web security, but it can be a bit thick and their site can be difficult to navigate.
Reply


Digg   Delicious   Reddit   Facebook   Twitter   StumbleUpon  


  Theme © 2014 iAndrew  
Powered By MyBB, © 2002-2019 MyBB Group.