From c776a1cd673ece41676b245fa64981eee8836063 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 15 Sep 2011 14:15:41 +0200 Subject: [PATCH] BUGFIX Consistently using Convert::raw2sql() instead of DB::getConn()->addslashes() or PHP's deprecated addslashes() for database escaping --- docs/en/topics/security.md | 6 ++++-- filesystem/Folder.php | 6 +++--- model/ComponentSet.php | 2 +- model/MySQLDatabase.php | 7 +++---- model/Versioned.php | 2 +- model/fieldtypes/Boolean.php | 2 +- model/fieldtypes/Decimal.php | 4 ++-- model/fieldtypes/Int.php | 4 ++-- 8 files changed, 17 insertions(+), 16 deletions(-) diff --git a/docs/en/topics/security.md b/docs/en/topics/security.md index 902980fd7..f162f6e53 100644 --- a/docs/en/topics/security.md +++ b/docs/en/topics/security.md @@ -16,8 +16,10 @@ See [http://shiflett.org/articles/sql-injection](http://shiflett.org/articles/sq ### Automatic escaping -SilverStripe automatically runs [addslashes()](http://php.net/addslashes) in DataObject::write() wherever possible. Data -is escaped when saving back to the database, not when writing to object-properties. +SilverStripe automatically escapes data in `[api:DataObject::write()]` wherever possible, +through database-specific methods (see `[api:Database->addslashes()]`). +For `[api:MySQLDatabase]`, this will be `[mysql_real_escape_string()](http://de3.php.net/mysql_real_escape_string)`. +Data is escaped when saving back to the database, not when writing to object-properties. * DataObject::get_by_id() * DataObject::update() diff --git a/filesystem/Folder.php b/filesystem/Folder.php index 0ad64b3ea..3fc9523d6 100755 --- a/filesystem/Folder.php +++ b/filesystem/Folder.php @@ -87,7 +87,7 @@ class Folder extends File { // First, merge any children that are duplicates $duplicateChildrenNames = DB::query("SELECT \"Name\" FROM \"File\" WHERE \"ParentID\" = $parentID GROUP BY \"Name\" HAVING count(*) > 1")->column(); if($duplicateChildrenNames) foreach($duplicateChildrenNames as $childName) { - $childName = DB::getConn()->addslashes($childName); + $childName = Convert::raw2sql($childName); // Note, we do this in the database rather than object-model; otherwise we get all sorts of problems about deleting files $children = DB::query("SELECT \"ID\" FROM \"File\" WHERE \"Name\" = '$childName' AND \"ParentID\" = $parentID")->column(); if($children) { @@ -194,10 +194,10 @@ class Folder extends File { if(Member::currentUser()) $ownerID = Member::currentUser()->ID; else $ownerID = 0; - $filename = DB::getConn()->addslashes($this->Filename . $name); + $filename = Convert::raw2sql($this->Filename . $name); if($className == 'Folder' ) $filename .= '/'; - $name = DB::getConn()->addslashes($name); + $name = Convert::raw2sql($name); DB::query("INSERT INTO \"File\" (\"ClassName\", \"ParentID\", \"OwnerID\", \"Name\", \"Filename\", \"Created\", \"LastEdited\", \"Title\") diff --git a/model/ComponentSet.php b/model/ComponentSet.php index 01684066d..75a30a771 100755 --- a/model/ComponentSet.php +++ b/model/ComponentSet.php @@ -152,7 +152,7 @@ class ComponentSet extends DataObjectSet { $extraKeys = $extraValues = ''; if($extraFields) foreach($extraFields as $k => $v) { $extraKeys .= ", \"$k\""; - $extraValues .= ", '" . DB::getConn()->addslashes($v) . "'"; + $extraValues .= ", '" . Convert::raw2sql($v) . "'"; } DB::query("INSERT INTO \"$this->tableName\" (\"$parentField\",\"$childField\" $extraKeys) VALUES ({$this->ownerObj->ID}, {$item->ID} $extraValues)"); diff --git a/model/MySQLDatabase.php b/model/MySQLDatabase.php index 9f5e70ca5..aace302b6 100755 --- a/model/MySQLDatabase.php +++ b/model/MySQLDatabase.php @@ -347,9 +347,9 @@ class MySQLDatabase extends SS_Database { if($field['Default'] || $field['Default'] === "0") { if(is_numeric($field['Default'])) - $fieldSpec .= " default " . addslashes($field['Default']); + $fieldSpec .= " default " . Convert::raw2sql($field['Default']); else - $fieldSpec .= " default '" . addslashes($field['Default']) . "'"; + $fieldSpec .= " default '" . Convert::raw2sql($field['Default']) . "'"; } if($field['Extra']) $fieldSpec .= " $field[Extra]"; @@ -866,8 +866,7 @@ class MySQLDatabase extends SS_Database { } /* - * This will return text which has been escaped in a database-friendly manner - * Using PHP's addslashes method won't work in MSSQL + * This will return text which has been escaped in a database-friendly manner. */ function addslashes($value){ return mysql_real_escape_string($value, $this->dbConn); diff --git a/model/Versioned.php b/model/Versioned.php index bf74321bd..9a646fb63 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -406,7 +406,7 @@ class Versioned extends DataExtension { // Add any extra, unchanged fields to the version record. $data = DB::query("SELECT * FROM \"$table\" WHERE \"ID\" = $id")->record(); if($data) foreach($data as $k => $v) { - if (!isset($newManipulation['fields'][$k])) $newManipulation['fields'][$k] = "'" . DB::getConn()->addslashes($v) . "'"; + if (!isset($newManipulation['fields'][$k])) $newManipulation['fields'][$k] = "'" . Convert::raw2sql($v) . "'"; } // Set up a new entry in (table)_versions diff --git a/model/fieldtypes/Boolean.php b/model/fieldtypes/Boolean.php index cb4b155e0..135416157 100644 --- a/model/fieldtypes/Boolean.php +++ b/model/fieldtypes/Boolean.php @@ -59,7 +59,7 @@ class Boolean extends DBField { */ function prepValueForDB($value) { if(strpos($value, '[')!==false) - return addslashes($value); + return Convert::raw2sql($value); else { if($value && strtolower($value) != 'f') { return "'1'"; diff --git a/model/fieldtypes/Decimal.php b/model/fieldtypes/Decimal.php index 08a1d33c5..57671b632 100644 --- a/model/fieldtypes/Decimal.php +++ b/model/fieldtypes/Decimal.php @@ -59,9 +59,9 @@ class Decimal extends DBField { if(strpos($value, '[')===false) return '0'; else - return addslashes($value); + return Convert::raw2sql($value); } else { - return addslashes($value); + return Convert::raw2sql($value); } } diff --git a/model/fieldtypes/Int.php b/model/fieldtypes/Int.php index 2f14a725e..4f34376d7 100644 --- a/model/fieldtypes/Int.php +++ b/model/fieldtypes/Int.php @@ -57,9 +57,9 @@ class Int extends DBField { if(strpos($value, '[')===false) return '0'; else - return addslashes($value); + return Convert::raw2sql($value); } else { - return addslashes($value); + return Convert::raw2sql($value); } }