atk4 / atk4-addons

Agile Toolkit Addons
http://atk4.com/
Other
15 stars 23 forks source link

Warning! Potential security vulnerability and data loss. #4

Closed jancha closed 12 years ago

jancha commented 13 years ago

addons/mvc/lib/Model/Table.php

we have method there function dsql($instance=null,$select_mode=true,$entity_code=null)

if one wants to use $model->dsql() and then perform non-select operation (e.g. do_delete()), false should be set as first param, as else tables are appended with table aliases and thus create illegal sql query.

$dq = $model->dsql(null, false);

now, if you previously in the model initialisation had used

$this->setMasterField($field, $value) //e.g. "user", "1"

and then if you have a method, which is performing cleanup in following way:

$dq->do_delete(); //assuming, that setMasterField is there

then you will have delete operation performed WITHOUT master field conditions.

if $select_mode is set to false, then conditions to dsql are not applied. thus do_delete would clean up all records in the db :)

this is what happened in the test environment in gradpool. so not big deal, but just be informed that setMasterField is dangerous!!!

potential solution: 1) add new method function applyMasterConditions($dq){ if ($this->init_where){ foreach ($this->init_where as $k => $v){ $dq->where($k, $v); } } return $dq; } 2) if you are using $model->dsql(null, false), then either inside function new_dsql() automate execution of applyMasterConditions, or add to manual to execute this manually. Obviously, if we have "Secure by default", this should happen automatically.

romaninsh commented 12 years ago

closing this down because it's too outdated and only applies to 4.1.x and was fixed in 4.2.0