SQL injection question

Yii 1.1 General Discussion
1

I’m not familiar with the innerworkings of CDbCriteria, so I’m curious about the protection against sql injection in this sort of instance. Am I safe?




// controller

$Criteria = new CDbCriteria();

$Criteria->condition = "`t`.`title` = $_GET['title']";

$records = MyActiveRecord::model()->findAll($Criteria);

~thinkt4nk

2

It is probably the worst possible solution! :)) But we are here to help:




$Criteria = new CDbCriteria();

$Criteria->condition = "`t`.`title` = :title";

$Criteria->params = array(':title'=>$_GET['title']);

$records = MyActiveRecord::model()->findAll($Criteria);

or shorter:




$records = MyActiveRecord::model()->findAll('title = :title', array(':title'=>$_GET['title']));

Never put GET parameters into conditions directly.

3

That’s what I figured. So, binding through params will sanitize the data? Thanks!

~thinkt4nk

4

fine, i understand

5

Ok, so what about these examples:




AR::model()->findAll('thisColumn=:thisColumn',array(':thisColumn'=>$_GET['someVar']));







ar->attribute = $_GET['someVar'];

ar->save();

What exactly does sanitize input? Is it just a CDbCriteria object? Is directly setting attributes without setAttributes() or ar->attributes = array(‘foo’=>‘bar’) safe to do? Sorry, just looking for some more clarification.

~thinkt4nk

6

so i need the params attribute for an addInCondition as well?

Good:


$criteria->addInCondition('category_id', ':categories');

$criteria->params = array(':categories' => $_POST['categories']);

Wrong:


$criteria->addInCondition('category_id', $_POST['categories']);
7

Is that an answer or a question?

~thinkt4nk

8

You need to understand what "prepared statements" are. Here is an article on the MySQL site: http://dev.mysql.com/tech-resources/articles/4.1/prepared-statements.html

Briefly, data is sent separately from queries, so it can’t screw up queries’ text.

I don’t think the second solution is wrong. I guess Yii creates placeholders automagically. Correct me if I’m wrong :)

9

Also check out PHP’s documentation of PDO. That’s what at the core of Yii’s DB abstraction layer:

http://de.php.net/manual/en/book.pdo.php

10

Cool, thx guys.

~thinkt4nk

11

Even if you bind your params you are NOT safe for xss attacks.

You need to filter the user input before you go with it to be saved into database.

If, for example, your $_POST[‘something’] = ‘<script>alert(1);</script>’; then even if you bind the param, this will not be safe, and when you echo it in frontend it will display the alert.

Use something like HTML Purifier or any other solid xss filter library to clean the input from users first, then after the input is cleaned of malicious code you can save it to database by binding the necessary params .

12

Brother, i am totally new in yii. So your above conversation, i didnt understanding. Please can you tell me my code below, is either sql proof or not? thanks<tanim>


	public function actionView()

	{

		$this->render('view',array(

			'model'=>$this->loadModel(),

		));

	}


	public function actionCreate()

	{

		$model=new Admin;

		// Uncomment the following line if AJAX validation is needed

		// $this->performAjaxValidation($model);

		if(isset($_POST['Admin']))

		{

			$model->attributes=$_POST['Admin'];

			if($model->save())

				$this->redirect(array('view','id'=>$model->id));

		}

		$this->render('create',array(

			'model'=>$model,

		));

	}
13

Yes, it is.

14

many many thanks for quick reply bro, :rolleyes:

15

If I’ve pulled up a row using a model, is it safe to set fields on the model directly from $_POST or $_GET and then save? Will Yii take care of DB escaping when I do the save? What about using the model methods with GET or POST?

How safe is this code?




$user = User::model()->findByPk($_POST['id']);

$user->name = $_POST['name'];

$user->age = $_POST['age'];

$user->save();

Thanks

16

I would be interested in how secure the findByPk() part of the code is. Since it uses CDbCriteria I would assume it to be safe.

What I did not expect though, is that the url “/whatever?objectId=14abc” still perfectly manages to load Object with ID 14, just as “/whatever?objectId=14” does… (in the code it is just $_GET[‘objectId’])

Or am I missing something here?

17

i am using yii 1.1.16 and i used Active records like find and findbyatt. with bind paras. but its not capable of protect me from ‘1’=’1′; — sql injection. what should i do?

18

It is capable if you’ve used binded params. If you’ve used concatenated strings, it’s considered a programmer mistake.