• 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
I have two tech arguing over whether this is a critical security problem

#1
Raised here, as its not a security problem with codeignitor, is a problem with how its being used.

Comments from one tech below.

The code I reviewed contained a critical security issue that might enable an attacker to steal or delete the whole database.

Here we are talking about the Codeigniter 3 model we reviewed together.
Codeigniter offers the posibility to write the whole query yourself like:

$this->db->query('SELECT * FROM some_table WHERE id = 3 AND status = 4 AND author = "Rick"');

in this case you need to actually write it more like (query binding and escaping to enforce security)

$sql = "SELECT * FROM some_table WHERE id = ? AND status = ? AND author = 'Rick'"
$this->db->query($sql, array($this->db->escape($id), $this->db->escape($status), $this->db->escape($author)));

Which I completely agreed with (Other tech guy) and I was aware of this.

But I am taking advantage of what the framework is offering me and that is called a Query Builder Class

So in my case the query would look like:

$wheresql = "id = {VAR}";

$wheresql .= "status = {VAR}";


$wheresql .= "author = {VAR}";

$this -> db -> where($wheresql);

The "where" method that is called on the db object is calling another method called _wh which is escaping the value or $wheresql

So I am more than sure there is no security problem in that. I already tried myself all "hacking" ways to see if I can inject something and I could not succeed.

Who is right?
is the database safe or not.
Reply

#2
(07-19-2017, 01:11 AM)nippi9 Wrote: Raised here, as its not a security problem with codeignitor, is a problem with how its being used.

Comments from one tech below.

The code I reviewed contained a critical security issue that might enable an attacker to steal or delete the whole database.

Here we are talking about the Codeigniter 3 model we reviewed together.
Codeigniter offers the posibility to write the whole query yourself like:

$this->db->query('SELECT * FROM some_table WHERE id = 3 AND status = 4 AND author = "Rick"');

in this case you need to actually write it more like (query binding and escaping to enforce security)

$sql = "SELECT * FROM some_table WHERE id = ? AND status = ? AND author = 'Rick'"
$this->db->query($sql, array($this->db->escape($id), $this->db->escape($status), $this->db->escape($author)));

Which I completely agreed with (Other tech guy) and I was aware of this.

But I am taking advantage of what the framework is offering me and that is called a Query Builder Class

So in my case the query would look like:

$wheresql = "id = {VAR}";

$wheresql .= "status = {VAR}";


$wheresql .= "author = {VAR}";

$this -> db -> where($wheresql);

The "where" method that is called on the db object is calling another method called _wh which is escaping the value or $wheresql

So I am more than sure there is no security problem in that. I already tried myself all "hacking" ways to see if I can inject something and I could not succeed.

Who is right?
is the database safe or not.

Hello! 
I think that you should have another mindset. No system in the world cannot be 100% safe. But it can be 99,8% safe depending on the risk to be hacked etc. 

If you only relying on CI's features you are not safe. 

Many security issues depending on the structure of code and database and user-levels and what every user can and cannot do. 

- How du you validating forms? 
- Who is accepted to fill in your forms? 
- How do you log your user activities
And so on. 

So more of your security is about structure of usage for your visitors and logging.so if you think your site is safe with just escaping you are wrong. 

And if you wonder why Im sounds negative is not that Im a negative person. But when it comes to online-security my mindset is quite negative. There is always a way in. The question is,  which way or (ways).
Reply

#3
As much as I agree with the above comments, I think that where you have done this:

PHP Code:
$wheresql .= "author = {VAR}";

$this -> db -> where($wheresql); 

What you are actually doing is creating a long complex sting (note the '.=') and expecting the where statement to know where and what to escape no matter how complex the where statement you are generating is.

As a simple rule, if you are generating an SQL statement (or partial statement) yourself, then escape the variables. If you are using the where statement as intended...

PHP Code:
$this->db->where('author'$author_identifier); 

Then you can rely on the query builder to escape this correctly.

So I agree with the tec that said to escape it if you are constructing your own SQL or partial SQL statements.

Hope that helps,

Paul.

PS I am not 100% certain that the query builder can escape complex queries reliably as you seem to have asked about, but I think that the general rule of escaping sql statements you make yourself is a valid point.
Reply

#4
If you use CI's parameterized queries (key = ?) you are safe AFAIK. Our app has been penetration tested 3 times but 3 different firms and they have never found a vector for SQL injection.
Reply

#5
If you do it that way, 

PHP Code:
$wheresql "id = {VAR}";
$wheresql .= "status = {VAR}";
$wheresql .= "author = {VAR}"

the $wheresql will contain

PHP Code:
"id = {VAR}status = {VAR}author = {VAR}";  

(am I right?)

plus, so far as I know, CodeIgniter query builder will automatically escape query if you do like this:

PHP Code:
$this->db->where('id'$id); 

so, if you make query without query builder, you must escape it with 

PHP Code:
$this->db->escape();  
Reply


Digg   Delicious   Reddit   Facebook   Twitter   StumbleUpon  


Users browsing this thread:
1 Guest(s)


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