Post by Goliath » Thu May 23, 2013 10:22 pm

What exactly does the $this->db->escape() line do?

I assume this executes htmlentities on the string?

-------------------------------------------------
Opencart v.1.5.4 (clean install)

New member

Posts

Joined
Mon May 20, 2013 6:19 am

Post by WilliamBD » Thu May 23, 2013 10:37 pm

This function determines the data type so that it can escape only string data. It also automatically adds single quotes around the data so you don't have to:
Reference: http://ellislab.com/codeigniter/user-gu ... eries.html

UPDATE:
It does the following exactly: (Found in system->database->mysql.php)

Code: Select all

	public function escape($value) {
		if ($this->link) {
			return mysql_real_escape_string($value, $this->link);
		}
	}
PHP Function: mysql_real_escape_string()
http://us.php.net/mysql%5Freal%5Fescape%5Fstring

*Disclaimer: I hope I have been as clear as possible and helpful. If you are unsure of anything please ask I will be happy to help - I do frequently watch the posts I have posted in previously.

How to change prices across a whole multi-store, with a baseline price for each product:
http://forum.opencart.com/viewtopic.php ... 24#p406793


Active Member

Posts

Joined
Fri Feb 08, 2013 9:33 pm

Post by Goliath » Fri May 24, 2013 6:58 am

Thanks for your answer.. I've got one more additional related question though.

I'm sure opencart is pretty safe, but what if I add some custom fields to the registration page.
Basicly the validation part of the controller register.php file only checks whether the variables ain't empty and then the variables are being written to the database, ofcourse escaped by the

Code: Select all

$this->db->escape($variable)
line.

Is this the only protection or am I overseeing something? And does this protection protect against sql injections, xss and so on?


sorry if this has been asked before, i'm kinda new to this forum.

New member

Posts

Joined
Mon May 20, 2013 6:19 am

Post by straightlight » Fri May 24, 2013 10:50 am

Escaping a variable upon SQL insertion is for mySQL server libraries to avoid confusion between static and dynamic functions when initiated from query strings.

Without escaping:

Code: Select all

test123
Escaping:

Code: Select all

\test123\
which also avoids confusion when using spaces since multiple languages, not just for PHP, does NOT use spaces when static functions are implied. ;)

Note: The addslashes function can also be used with PHP when using other database libraries.

The most generated errors being found on Opencart forum originates from contributed programming. The increased post counters are caused by redundancies of the same solutions that were already provided prior.


Regards,
Straightlight
Opencart.com Administrator / Quality Assurance Analyst / Programmer


Legendary Member

Posts

Joined
Mon Nov 14, 2011 11:38 pm
Location - Canada, ON

Post by rph » Fri May 24, 2013 4:48 pm

Goliath wrote:Is this the only protection or am I overseeing something? And does this protection protect against sql injections, xss and so on?
The database escape method is only useful if you've enclosed the variable with single quotes. If you don't then you're still susceptible to SQL injection.

http://johnroach.info/2011/02/17/why-my ... n-attacks/

-Ryan


rph
Expert Member

Posts

Joined
Fri Jan 08, 2010 5:05 am
Location - Lincoln, Nebraska

Post by WilliamBD » Fri May 24, 2013 4:54 pm

Goliath wrote:Thanks for your answer.. I've got one more additional related question though.

I'm sure opencart is pretty safe, but what if I add some custom fields to the registration page.
Basicly the validation part of the controller register.php file only checks whether the variables ain't empty and then the variables are being written to the database, ofcourse escaped by the

Code: Select all

$this->db->escape($variable)
line.

Is this the only protection or am I overseeing something? And does this protection protect against sql injections, xss and so on?
I will go through what register.php does

Code: Select all

if (($this->request->server['REQUEST_METHOD'] == 'POST') && $this->validate()) {
This checks if the form was submitted and if the function validate passes.

The function validate: (Example of first name)

Code: Select all

if ((utf8_strlen($this->request->post['firstname']) < 1) || (utf8_strlen($this->request->post['firstname']) > 32)) {
This checks the sting length in utf8 and if its length is between 1 and 32. (FYI <script></script> in utf is around 30-31). Injected code use a lot more characters in utf8.

And then the code is escaped in the db.

As for

Code: Select all

mysql_real_escape_string()
if your server is using sql 5.1 (late one wards) its as far as I know secure.
-----
Security is a huge thing for an online shop and if you search the forums you will find this topic has been bought up several times. Reputation for having weak security (just google famous ones) is a killer for an application like this. Which is one of the reasons I believe Daniel will only allow certain people add/change code. It slows down development obviously with one or two people working on the code. But thats whilst a weakness is also its security strength.

*This is just opinion and I do not speak for Daniel or know of his intentions.

The last security update I could find with a quick search is:
http://forum.opencart.com/viewtopic.php?t=62257

*Disclaimer: I hope I have been as clear as possible and helpful. If you are unsure of anything please ask I will be happy to help - I do frequently watch the posts I have posted in previously.

How to change prices across a whole multi-store, with a baseline price for each product:
http://forum.opencart.com/viewtopic.php ... 24#p406793


Active Member

Posts

Joined
Fri Feb 08, 2013 9:33 pm

Post by WilliamBD » Fri May 24, 2013 4:59 pm

rph wrote:
Goliath wrote:Is this the only protection or am I overseeing something? And does this protection protect against sql injections, xss and so on?
The database escape method is only useful if you've enclosed the variable with single quotes. If you don't then you're still susceptible to SQL injection.

http://johnroach.info/2011/02/17/why-my ... n-attacks/
Which can be seen in the 'model->account->customer.php' addCustomer() function.


Code: Select all

$this->db->query("INSERT INTO " . DB_PREFIX . "customer SET store_id = '" . (int)$this->config->get('config_store_id') . "', firstname = '" . $this->db->escape($data['firstname']) . "', lastname = '" . $this->db->escape($data['lastname']) . "', email = '" . $this->db->escape($data['email']) . "', telephone = '" . $this->db->escape($data['telephone']) . "', fax = '" . $this->db->escape($data['fax']) . "', salt = '" . $this->db->escape($salt = substr(md5(uniqid(rand(), true)), 0, 9)) . "', password = '" . $this->db->escape(sha1($salt . sha1($salt . sha1($data['password'])))) . "', newsletter = '" . (isset($data['newsletter']) ? (int)$data['newsletter'] : 0) . "', customer_group_id = '" . (int)$customer_group_id . "', ip = '" . $this->db->escape($this->request->server['REMOTE_ADDR']) . "', status = '1', approved = '" . (int)!$customer_group_info['approval'] . "', date_added = NOW()");
More specifically:

Code: Select all

...'" . $this->db->escape($data['firstname']) . "'...

*Disclaimer: I hope I have been as clear as possible and helpful. If you are unsure of anything please ask I will be happy to help - I do frequently watch the posts I have posted in previously.

How to change prices across a whole multi-store, with a baseline price for each product:
http://forum.opencart.com/viewtopic.php ... 24#p406793


Active Member

Posts

Joined
Fri Feb 08, 2013 9:33 pm

Post by Goliath » Fri May 24, 2013 7:08 pm

Thanks for all the answers, I really appreciate it.

So let's say I want to build in a textarea within the registration form, so people can tell about themselves (for example).

And I write this in the validation:

Code: Select all

    	if ((utf8_strlen($this->request->post['exampletext']) < 1) || (utf8_strlen($this->request->post['exampletext']) > 255)) {
      		$this->error['exampletext'] = $this->language->get('error_exampletext');
    	}
And I write it to the database with:

Code: Select all

$this->db->escape($this->request->post['exampletext'])
Will that be safe ? If not what do I have to add to make it safe?

Ps: please note the 255 characters in the validation part

New member

Posts

Joined
Mon May 20, 2013 6:19 am

Post by WilliamBD » Fri May 24, 2013 8:34 pm

Pretty much.

You will need to add this field/column to the database as well.

Where you want to display this information is something you may have not had a chance to think about yet. But this will involve changing some database query code.

*Disclaimer: I hope I have been as clear as possible and helpful. If you are unsure of anything please ask I will be happy to help - I do frequently watch the posts I have posted in previously.

How to change prices across a whole multi-store, with a baseline price for each product:
http://forum.opencart.com/viewtopic.php ... 24#p406793


Active Member

Posts

Joined
Fri Feb 08, 2013 9:33 pm

Post by straightlight » Fri May 24, 2013 9:51 pm

Will that be safe ? If not what do I have to add to make it safe?
Text area fields in the database can be tricky sometimes when inserting data into the database. There can be many ways to insert data, in that case.

Adding images into the database is, of course, different in OC. It would be done like this:

Code: Select all

$this->db->escape(html_entity_decode($this->request->post['image'], ENT_QUOTES, 'UTF-8'))

The most generated errors being found on Opencart forum originates from contributed programming. The increased post counters are caused by redundancies of the same solutions that were already provided prior.


Regards,
Straightlight
Opencart.com Administrator / Quality Assurance Analyst / Programmer


Legendary Member

Posts

Joined
Mon Nov 14, 2011 11:38 pm
Location - Canada, ON

Post by Goliath » Fri May 24, 2013 10:21 pm

straightlight wrote:
Text area fields in the database can be tricky sometimes when inserting data into the database. There can be many ways to insert data, in that case.
Yeah I know, I think I'm gonna play it safe and add strip_tags to it.. like this:

Code: Select all

 $this->db->escape(strip_tags(trim($this->request->post['exampletext'])))

New member

Posts

Joined
Mon May 20, 2013 6:19 am

Post by straightlight » Sat May 25, 2013 12:36 am

This would be the one you'd be looking for since OpenCart uses utf8_bin structure to insert data into the database:

Code: Select all

$this->db->escape(strip_tags(html_entity_decode(trim($this->request->post['exampletext']), ENT_QUOTES, 'UTF-8'))

The most generated errors being found on Opencart forum originates from contributed programming. The increased post counters are caused by redundancies of the same solutions that were already provided prior.


Regards,
Straightlight
Opencart.com Administrator / Quality Assurance Analyst / Programmer


Legendary Member

Posts

Joined
Mon Nov 14, 2011 11:38 pm
Location - Canada, ON

Post by rph » Sat May 25, 2013 12:54 am

Goliath wrote:And I write it to the database with:

Code: Select all

$this->db->escape($this->request->post['exampletext'])
Will that be safe ? If not what do I have to add to make it safe?

Ps: please note the 255 characters in the validation part
Just make sure to enclose your data with single quotes.

Code: Select all

$this->db->query("INSERT INTO `" . DB_PREFIX . "table` SET text = '" . $this->db->escape($this->request->post['exampletext']) . "'");
If it's not enclosed an attacker could submit:

Code: Select all

1; UPDATE user SET password = 123;
You might also be interested to know the Request class automatically applies htmlspecialchars():

Code: Select all

<?php
class Request {
	public $get = array();
	public $post = array();
	public $cookie = array();
	public $files = array();
	public $server = array();

	public function __construct() {
		$_GET = $this->clean($_GET);
		$_POST = $this->clean($_POST);
		$_REQUEST = $this->clean($_REQUEST);
		$_COOKIE = $this->clean($_COOKIE);
		$_FILES = $this->clean($_FILES);
		$_SERVER = $this->clean($_SERVER);

		$this->get = $_GET;
		$this->post = $_POST;
		$this->request = $_REQUEST;
		$this->cookie = $_COOKIE;
		$this->files = $_FILES;
		$this->server = $_SERVER;
	}

	public function clean($data) {
		if (is_array($data)) {
			foreach ($data as $key => $value) {
				unset($data[$key]);

				$data[$this->clean($key)] = $this->clean($value);
			}
		} else { 
			$data = htmlspecialchars($data, ENT_COMPAT, 'UTF-8');
		}

		return $data;
	}
}
?>

-Ryan


rph
Expert Member

Posts

Joined
Fri Jan 08, 2010 5:05 am
Location - Lincoln, Nebraska

Post by Goliath » Sat May 25, 2013 1:28 am

Okay, I'm gonna use this:

Code: Select all

$this->db->query("INSERT INTO `" . DB_PREFIX . "table` SET text = '" . $this->db->escape(strip_tags(html_entity_decode(trim($this->request->post['exampletext']), ENT_QUOTES, 'UTF-8')) . "'");
Thanks everybody!
Have a nice weekend!
;)

New member

Posts

Joined
Mon May 20, 2013 6:19 am

Post by straightlight » Sun May 26, 2013 12:19 am

Code: Select all

$this->db->query("INSERT INTO `" . DB_PREFIX . "table` SET `text` = '" . $this->db->escape(strip_tags(html_entity_decode(trim($this->request->post['exampletext']), ENT_QUOTES, 'UTF-8')) . "'");

The most generated errors being found on Opencart forum originates from contributed programming. The increased post counters are caused by redundancies of the same solutions that were already provided prior.


Regards,
Straightlight
Opencart.com Administrator / Quality Assurance Analyst / Programmer


Legendary Member

Posts

Joined
Mon Nov 14, 2011 11:38 pm
Location - Canada, ON

Post by Goliath » Thu May 30, 2013 10:34 pm

@straightlight

I'm getting an sql error with your example..
I've got this coding:

Code: Select all

$this->db->query("INSERT INTO `" . DB_PREFIX . "table` SET  `text` = '" . $this->db->escape(strip_tags(html_entity_decode(trim($this->request->post['exampletext']), ENT_QUOTES, 'UTF-8')) . "'"));   
Where exampletext contains 60 (value submitted by the user) then I'm getting this error:

Code: Select all

Notice: Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''60\'' at line 1
Error No: 1064
INSERT INTO `table` SET  `text` = '60\'
How can I solve this sql error?

New member

Posts

Joined
Mon May 20, 2013 6:19 am

Post by rph » Fri May 31, 2013 12:54 am

The code is bugged.

-Ryan


rph
Expert Member

Posts

Joined
Fri Jan 08, 2010 5:05 am
Location - Lincoln, Nebraska

Post by straightlight » Fri May 31, 2013 12:58 am

I just tried with:

Code: Select all

$this->db->query("INSERT INTO `" . DB_PREFIX . "table` SET `text` = '" . $this->db->escape(html_entity_decode(trim(strip_tags($this->request->post['exampletext'])), ENT_QUOTES, 'UTF-8')) . "'");
I don't get any errors. If you still get errors with this replacement, would it be possible to know your PHP version?

The most generated errors being found on Opencart forum originates from contributed programming. The increased post counters are caused by redundancies of the same solutions that were already provided prior.


Regards,
Straightlight
Opencart.com Administrator / Quality Assurance Analyst / Programmer


Legendary Member

Posts

Joined
Mon Nov 14, 2011 11:38 pm
Location - Canada, ON

Post by rph » Fri May 31, 2013 2:29 am

straightlight wrote:I don't get any errors.
Yeah, well, I'm not surprised there. Take two seconds and line up the parenthesis in the original code you posted.

-Ryan


rph
Expert Member

Posts

Joined
Fri Jan 08, 2010 5:05 am
Location - Lincoln, Nebraska

Post by Goliath » Fri May 31, 2013 6:58 am

it works perfectly now, thanks!!

New member

Posts

Joined
Mon May 20, 2013 6:19 am
Who is online

Users browsing this forum: No registered users and 81 guests