fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

BC break: fixed potential vulnerability in quoting identifies in a SQL command #1707

Closed kenjis closed 10 years ago

kenjis commented 10 years ago

https://github.com/fuel/core/commit/7574d5c78aae52ecdbedd138b2614bb2345ecff5 made the below code error.

    public static function _validation_unique($val, $options)
    {
        list($table, $field) = explode('.', $options);

        $result = DB::select("LOWER (\"$field\")")
        ->where($field, '=', Str::lower($val))
        ->from($table)->execute();

        return ! ($result->count() > 0);
    }

http://fuelphp.com/docs/classes/validation/validation.html

Fuel\Core\Database_Exception [ Error ]:
SQLSTATE[42S22]: Column not found: 1054 Unknown column 'LOWER ("name")' in 'field list' with query: "SELECT `LOWER ("name")` FROM `table` WHERE `name` = 'foo'"
WanWizard commented 10 years ago

I was afraid this would popup...

WanWizard commented 10 years ago

It's a docs error, fixed

kenjis commented 10 years ago

You mean it is a BC break change, because of security issue?

Would you please add to Changelog? https://github.com/fuel/core/wiki/Changelog-v1.7.2#backward-compatibility-notes

WanWizard commented 10 years ago

No, SQL functions should have been coded using DB::expr(), but this example didn't. The example worked because of the issue, and now that the issue is fixed, the example was broken. So the correct thing to do was fix the example.

I'll make a note of it in the changelog.

WanWizard commented 10 years ago

Done: https://github.com/fuel/core/wiki/Changelog-v1.7.2#database

kenjis commented 10 years ago

Thanks!