Skip to content
This repository has been archived by the owner on Sep 4, 2022. It is now read-only.

Warning! Potential security vulnerability and data loss. #4

Closed
jancha opened this issue Feb 17, 2011 · 1 comment
Closed

Warning! Potential security vulnerability and data loss. #4

jancha opened this issue Feb 17, 2011 · 1 comment

Comments

@jancha
Copy link
Contributor

jancha commented Feb 17, 2011

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
Copy link
Member

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants