is \Base::clean enough for sanitization?

[AM]

Hi,

is \Base::clean 100% secure for sanitizing user input?

thanks

[Anatol Buchholz]

Do you ask if it does this automatically or if it´s provided methods are bullet proof?

[vijinho]

It should be.  Maybe it's overkill but I do this to clean and normalize all input:

// clean ALL incoming user input by default $request = []; $utf = \UTF::instance(); foreach (['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'COOKIE'] as $var) { $f3->copy($var, $var . '_UNCLEAN'); $input = $f3->get($var); if (is_array($input) && count($input)) { $cleaned = []; foreach ($input as $k => $v) { $cleaned[strtolower($utf->trim($f3->clean($k)))] = $f3->recursive($v, function ($v) use ($f3, $utf) { return $utf->trim($f3->clean($v)); }); } ksort($cleaned); $request = array_merge_recursive($request, $cleaned); $f3->set($var, $cleaned); } } unset($cleaned); // we don't want to include the session name in the request data $session_name = strtolower(session_name()); if (array_key_exists($session_name, $request)) { unset($request[$session_name]); } ksort($request); $f3->copy('REQUEST', 'REQUEST_UNCLEAN'); $f3->set('REQUEST', $request); unset($request);

[Rayne]

It isn't secure: https://github.com/bcosca/fatfree/issues/850

[vijinho]

That's interesting.  I also use https://github.com/Wixel/GUMP as a second layer of filtering (and validation) before any fields are updated within my database.

[AM]

Well, if I do something like this:

$item = new \DB\SQL\Mapper($db, 'item');
$item->variable =$f3->get('POST.variable');
$item->save();

1. Could the database be hacked using that code by a SQL iinjection or something?

2. Is this safe for output when the output is being escaped (which is the default in the f3 template engine)?

Thanks

[Anatol Buchholz]


$item = new \DB\SQL\Mapper($db, 'item');
$item->variable =$f3->get('POST.variable');
$item->save();


1. Could the database be hacked using that code by a SQL iinjection or something?

I think yes and would suggest to sanatize data and/or use parameterized queries

It is recommended to use parameterized queries for all where conditions that may include user input data.

 

https://fatfreeframework.com/sql-mapper#Syntax 

2. Is this safe for output when the output is being escaped (which is the default in the f3 template engine)?

Yes, in past projects I´ve trusted F3 here: https://fatfreeframework.com/views-and-templates#DataSanitation

 

[AM]

Am Sonntag, 11. September 2016 14:49:36 UTC+2 schrieb Anatol Buchholz:


$item = new \DB\SQL\Mapper($db, 'item');
$item->variable =$f3->get('POST.variable');
$item->save();


1. Could the database be hacked using that code by a SQL iinjection or something?

I think yes and would suggest to sanatize data and/or use parameterized queries

It is recommended to use parameterized queries for all where conditions that may include user input data.

 

https://fatfreeframework.com/sql-mapper#Syntax 

 I would have thought that F3 internally uses parameterized queries when using $mapper->save()?

it seems it does?

See \DB\SQL\Mapper::update(), line 456:

foreach ($this->fields as $key=>$field)

if ($field['changed']) {

$pairs.=($pairs?',':'').$this->db->quotekey($key).'=?';

$args[$ctr+1]=array($field['value'],$field['pdo_type']);

$ctr++;

}

[ikkez]

yes it does.
this is not the point where the bind happens. it's here:
https://github.com/bcosca/fatfree-core/blob/master/db/sql.php#L201

the mapper always uses prepared statements, which is relativly safe against sql injections. The only point where you should be careful is the $options array you can put into for limit, offset, sorting and group. these things cannot use prepared statements, so better check twice if you want to use dynamic values here.

[AM]

Great!

Thanks ikkez!