Skip to content

Commit

Permalink
BUGFIX Consistently using Convert::raw2sql() instead of DB::getConn()…
Browse files Browse the repository at this point in the history
…->addslashes() or PHP's deprecated addslashes() for database escaping
  • Loading branch information
chillu committed Sep 15, 2011
1 parent 01b08a5 commit 73cca09
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 17 deletions.
2 changes: 1 addition & 1 deletion core/model/ComponentSet.php
Expand Up @@ -152,7 +152,7 @@ protected function loadChildIntoDatabase($item, $extraFields = null) {
$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)");
Expand Down
7 changes: 3 additions & 4 deletions core/model/MySQLDatabase.php
Expand Up @@ -347,9 +347,9 @@ public function fieldList($table) {

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]";

Expand Down Expand Up @@ -862,8 +862,7 @@ function dbDataType($type){
}

/*
* 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);
Expand Down
2 changes: 1 addition & 1 deletion core/model/Versioned.php
Expand Up @@ -392,7 +392,7 @@ function augmentWrite(&$manipulation) {
// 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
Expand Down
2 changes: 1 addition & 1 deletion core/model/fieldtypes/Boolean.php
Expand Up @@ -59,7 +59,7 @@ public function scaffoldSearchField($title = null) {
*/
function prepValueForDB($value) {
if(strpos($value, '[')!==false)
return addslashes($value);
return Convert::raw2sql($value);
else {
if($value && strtolower($value) != 'f') {
return "'1'";
Expand Down
4 changes: 2 additions & 2 deletions core/model/fieldtypes/Decimal.php
Expand Up @@ -59,9 +59,9 @@ function prepValueForDB($value) {
if(strpos($value, '[')===false)
return '0';
else
return addslashes($value);
return Convert::raw2sql($value);
} else {
return addslashes($value);
return Convert::raw2sql($value);
}
}

Expand Down
4 changes: 2 additions & 2 deletions core/model/fieldtypes/Int.php
Expand Up @@ -57,9 +57,9 @@ function prepValueForDB($value) {
if(strpos($value, '[')===false)
return '0';
else
return addslashes($value);
return Convert::raw2sql($value);
} else {
return addslashes($value);
return Convert::raw2sql($value);
}
}

Expand Down
6 changes: 4 additions & 2 deletions docs/en/topics/security.md
Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions filesystem/Folder.php
Expand Up @@ -77,7 +77,7 @@ function syncChildren() {
// 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) {
Expand Down Expand Up @@ -184,10 +184,10 @@ function constructChild($name) {
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\")
Expand Down
2 changes: 1 addition & 1 deletion search/AdvancedSearchForm.php
Expand Up @@ -82,7 +82,7 @@ public function getResults($numPerPage = 10) {
foreach($_REQUEST['OnlyShow'] as $section => $checked) {
$items = explode(",", $section);
foreach($items as $item) {
$page = DataObject::get_one('SiteTree', "\"URLSegment\" = '" . DB::getConn()->addslashes($item) . "'");
$page = DataObject::get_one('SiteTree', "\"URLSegment\" = '" . Convert::raw2sql($item) . "'");
$pageList[] = $page->ID;
if(!$page) user_error("Can't find a page called '$item'", E_USER_WARNING);
$page->loadDescendantIDListInto($pageList);
Expand Down

0 comments on commit 73cca09

Please sign in to comment.