tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

MINOR Updated security documentation (phpdoc and markdown) around new DataList and SQLQuery APIs, additional automatic escaping on some stricter inputs like column names

Browse Source
This commit is contained in:
Ingo Schommer 2012-05-16 11:29:49 +02:00
parent 51eb70781b
commit 3a11c690ed
5 changed files with 153 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/en/misc/coding-conventions.md
View File

@ -425,7 +425,7 @@ Put code into the classes in the following order (where applicable).
### SQL Format
To make sure your code works across databases make sure you escape your queries like below,
If you have to use raw SQL, make sure your code works across databases make sure you escape your queries like below,
with the column or table name escaped with double quotes and values with single quotes.
:::php

37
docs/en/topics/security.md
View File

@ -2,9 +2,7 @@
## Introduction
This page details notes on how to ensure that we develop secure SilverStripe applications. See [security](/topics/security)
for the Silverstripe-class as a starting-point for most security-related functionality.
This page details notes on how to ensure that we develop secure SilverStripe applications.
See our "[Release Process](/misc/release-process#security-releases) on how to report security issues.
## SQL Injection
@ -16,11 +14,11 @@ See [http://shiflett.org/articles/sql-injection](http://shiflett.org/articles/sq
### Automatic escaping
SilverStripe automatically escapes data in `[api:DataObject::write()]` wherever possible,
SilverStripe automatically escapes data in SQL statements 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.
* Most `[api:DataList]` accessors (see escaping note in method documentation)
* DataObject::get_by_id()
* DataObject::update()
* DataObject::castedUpdate()
@ -30,6 +28,18 @@ Data is escaped when saving back to the database, not when writing to object-pro
* FormField->saveInto()
* DBField->saveInto()
Data is escaped when saving back to the database, not when writing to object-properties.
Example:
:::php
// automatically escaped/quoted
$members = Member::get()->filter('Name', $_GET['name']);
// automatically escaped/quoted
$members = Member::get()->filter(array('Name' => $_GET['name']));
// needs to be escaped/quoted manually
$members = Member::get()->where(sprintf('"Name" = \'%s\'', Convert::raw2sql($_GET['name'])));
<div class="warning" markdown='1'>
It is NOT good practice to "be sure" and convert the data passed to the functions below manually. This might
result in *double escaping* and alters the actually saved data (e.g. by adding slashes to your content).
@ -41,12 +51,13 @@ As a rule of thumb, whenever you're creating raw queries (or just chunks of SQL)
yourself. See [coding-conventions](/misc/coding-conventions) and [datamodel](/topics/datamodel) for ways to cast and convert
your data.
* SQLQuery
* DataObject::buildSQL()
* DB::query()
* Director::urlParams()
* Controller->requestParams, Controller->urlParams
* GET/POST data passed to a Form-method
* `SQLQuery`
* `DataObject::buildSQL()`
* `DB::query()`
* `Director::urlParams()`
* `Controller->requestParams`, `Controller->urlParams`
* `SS_HTTPRequest` data
* GET/POST data passed to a form method
Example:
@ -60,7 +71,7 @@ Example:
}
* FormField->Value()
* `FormField->Value()`
* URLParams passed to a Controller-method
Example:
@ -95,13 +106,11 @@ This means if you've got a chain of functions passing data through, escaping sho
}
}
This might not be applicable in all cases - especially if you are building an API thats likely to be customized. If
you're passing unescaped data, make sure to be explicit about it by writing *phpdoc*-documentation and *prefixing* your
variables ($RAW_data instead of $data).
## XSS (Cross-Site-Scripting)
SilverStripe helps you guard any output against clientside attacks initiated by malicious user input, commonly known as

26
model/DataList.php
View File

@ -87,7 +87,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
/**
* Add a WHERE clause to the query.
*
* @param string $filter
* @param string $filter Escaped SQL statement
* @return DataList
*/
public function where($filter) {
@ -120,7 +120,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
/**
* Add an join clause to this data list's query.
*
* @param type $join
* @param type $join Escaped SQL statement
* @return DataList
* @deprecated 3.0
*/
@ -133,7 +133,8 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
/**
* Restrict the records returned in this query by a limit clause
*
* @param string $limit
* @param int $limit
* @param int $offset
*/
public function limit($limit, $offset = 0) {
if(!$limit && !$offset) {
@ -151,12 +152,12 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
*
* @see SS_List::sort()
* @see SQLQuery::orderby
*
* @example $list->sort('Name'); // default ASC sorting
* @example $list->sort('Name DESC'); // DESC sorting
* @example $list->sort('Name', 'ASC');
* @example $list->sort(array('Name'=>'ASC,'Age'=>'DESC'));
*
* @param String|array Escaped SQL statement. If passed as array, all keys and values are assumed to be escaped.
* @return DataList
*/
public function sort() {
@ -215,6 +216,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
*
* @todo extract the sql from $customQuery into a SQLGenerator class
*
* @param String|array Escaped SQL statement. If passed as array, all keys and values are assumed to be escaped.
* @return DataList
*/
public function filter() {
@ -305,6 +307,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
*
* @todo extract the sql from this method into a SQLGenerator class
*
* @param String|array Escaped SQL statement. If passed as array, all keys and values are assumed to be escaped.
* @return DataList
*/
public function exclude(){
@ -356,8 +359,8 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
/**
* Add an inner join clause to this data list's query.
*
* @param string $table
* @param string $onClause
* @param string $table Table name (unquoted)
* @param string $onClause Escaped SQL statement, e.g. '"Table1"."ID" = "Table2"."ID"'
* @param string $alias - if you want this table to be aliased under another name
* @return DataList
*/
@ -370,8 +373,8 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
/**
* Add an left join clause to this data list's query.
*
* @param string $table
* @param string $onClause
* @param string $table Table name (unquoted)
* @param string $onClause Escaped SQL statement, e.g. '"Table1"."ID" = "Table2"."ID"'
* @param string $alias - if you want this table to be aliased under another name
* @return DataList
*/
@ -574,9 +577,9 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
if($key == 'ID') {
$baseClass = ClassInfo::baseDataClass($this->dataClass);
$SQL_col = "\"$baseClass\".\"$key\"";
$SQL_col = sprintf('"%s"."%s"', $baseClass, Convert::raw2sql($key));
} else {
$SQL_col = "\"$key\"";
$SQL_col = sprintf('"%s"', Convert::raw2sql($key));
}
return $clone->where("$SQL_col = '" . Convert::raw2sql($value) . "'")->First();
@ -597,10 +600,11 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
/**
* Filter this list to only contain the given Primary IDs
*
* @param array $ids
* @param array $ids Array of integers, will be automatically cast/escaped.
* @return DataList
*/
public function byIDs(array $ids) {
$ids = array_map('intval', $ids); // sanitize
$baseClass = ClassInfo::baseDataClass($this->dataClass);
$this->where("\"$baseClass\".\"ID\" IN (" . implode(',', $ids) .")");

86
model/DataQuery.php
View File

@ -321,30 +321,38 @@ class DataQuery {
/**
* Return the maximum value of the given field in this DataList
*
* @param String $field Unquoted database column name (will be escaped automatically)
*/
function Max($field) {
return $this->getFinalisedQuery()->aggregate("MAX(\"$field\")")->execute()->value();
return $this->getFinalisedQuery()->aggregate(sprintf('MAX("%s")', Convert::raw2sql($field))->execute()->value();
}
/**
* Return the minimum value of the given field in this DataList
*
* @param String $field Unquoted database column name (will be escaped automatically)
*/
function Min($field) {
return $this->getFinalisedQuery()->aggregate("MIN(\"$field\")")->execute()->value();
return $this->getFinalisedQuery()->aggregate(sprintf('MIN("%s")', Convert::raw2sql($field))->execute()->value();
}
/**
* Return the average value of the given field in this DataList
*
* @param String $field Unquoted database column name (will be escaped automatically)
*/
function Avg($field) {
return $this->getFinalisedQuery()->aggregate("AVG(\"$field\")")->execute()->value();
return $this->getFinalisedQuery()->aggregate(sprintf('AVG("%s")', Convert::raw2sql($field))->execute()->value();
}
/**
* Return the sum of the values of the given field in this DataList
*
* @param String $field Unquoted database column name (will be escaped automatically)
*/
function Sum($field) {
return $this->getFinalisedQuery()->aggregate("SUM(\"$field\")")->execute()->value();
return $this->getFinalisedQuery()->aggregate((sprintf('SUM("%s")', Convert::raw2sql($field)))->execute()->value();
}
/**
@ -392,6 +400,8 @@ class DataQuery {
/**
* Set the HAVING clause of this query
*
* @param String $having Escaped SQL statement
*/
function having($having) {
if($having) {
@ -404,7 +414,18 @@ class DataQuery {
}
/**
* Set the WHERE clause of this query
* Set the WHERE clause of this query.
* There are two different ways of doing this:
*
* <code>
* // the entire predicate as a single string
* $query->where("Column = 'Value'");
*
* // multiple predicates as an array
* $query->where(array("Column = 'Value'", "Column != 'Value'"));
* </code>
*
* @param string|array $where Predicate(s) to set, as escaped SQL statements.
*/
function where($filter) {
if($filter) {
@ -417,11 +438,13 @@ class DataQuery {
}
/**
* Set a WHERE with OR
* Set a WHERE with OR.
*
* @param array $filter
* @return DataQuery
* @example $dataQuery->whereAny(array("Monkey = 'Chimp'", "Color = 'Brown'"));
* @see where()
*
* @param array $filter Escaped SQL statement.
* @return DataQuery
*/
function whereAny($filter) {
if($filter) {
@ -438,6 +461,9 @@ class DataQuery {
*
* @see SQLQuery::orderby()
*
* @param String $sort Column to sort on (escaped SQL statement)
* @param String $direction Direction ("ASC" or "DESC", escaped SQL statement)
* @param Boolean $clear Clear existing values
* @return DataQuery
*/
function sort($sort = null, $direction = null, $clear = true) {
@ -464,7 +490,10 @@ class DataQuery {
}
/**
* Set the limit of this query
* Set the limit of this query.
*
* @param int $limit
* @param int $offset
*/
function limit($limit, $offset = 0) {
$clone = $this;
@ -494,9 +523,11 @@ class DataQuery {
}
/**
* Add an INNER JOIN clause to this queyr
* @param $table The table to join to.
* @param $onClause The filter for the join.
* Add an INNER JOIN clause to this query.
*
* @param String $table The unquoted table name to join to.
* @param String $onClause The filter for the join (escaped SQL statement)
* @param String $alias An optional alias name (unquoted)
*/
public function innerJoin($table, $onClause, $alias = null) {
if($table) {
@ -509,9 +540,11 @@ class DataQuery {
}
/**
* Add a LEFT JOIN clause to this queyr
* @param $table The table to join to.
* @param $onClause The filter for the join.
* Add a LEFT JOIN clause to this query.
*
* @param String $table The unquoted table to join to.
* @param String $onClause The filter for the join (escaped SQL statement).
* @param String $alias An optional alias name (unquoted)
*/
public function leftJoin($table, $onClause, $alias = null) {
if($table) {
@ -528,7 +561,7 @@ class DataQuery {
* mappings to the query object state. This has to be called
* in any overloaded {@link SearchFilter->apply()} methods manually.
*
* @param $relation The array/dot-syntax relation to follow
* @param String|array $relation The array/dot-syntax relation to follow
* @return The model class of the related item
*/
function applyRelation($relation) {
@ -619,11 +652,15 @@ class DataQuery {
}
/**
* Select the given fields from the given table
* Select the given fields from the given table.
*
* @param String $table Unquoted table name (will be escaped automatically)
* @param Array $fields Database column names (will be escaped automatically)
*/
public function selectFromTable($table, $fields) {
$table = Convert::raw2sql($table);
$fieldExpressions = array_map(create_function('$item',
"return '\"$table\".\"' . \$item . '\"';"), $fields);
"return '\"$table\".\"' . Convert::raw2sql(\$item) . '\"';"), $fields);
$this->query->setSelect($fieldExpressions);
@ -632,6 +669,8 @@ class DataQuery {
/**
* Query the given field column from the database and return as an array.
*
* @param String $field See {@link expressionForField()}.
*/
public function column($field = 'ID') {
$query = $this->getFinalisedQuery(array($field));
@ -644,6 +683,12 @@ class DataQuery {
return $query->execute()->column($field);
}
/**
* @param String $field Select statement identifier, either the unquoted column name,
* the full composite SQL statement, or the alias set through {@link SQLQquery->selectField()}.
* @param SQLQuery $query
* @return String
*/
protected function expressionForField($field, $query) {
// Special case for ID
if($field == 'ID') {
@ -656,7 +701,10 @@ class DataQuery {
}
/**
* Select the given field expressions. You must do your own escaping
* Select the given field expressions.
*
* @param $fieldExpression String The field to select (escaped SQL statement)
* @param $alias String The alias of that field (escaped SQL statement)
*/
protected function selectField($fieldExpression, $alias = null) {
$this->query->selectField($fieldExpression, $alias);

80
model/SQLQuery.php
View File

@ -204,8 +204,9 @@ class SQLQuery {
/**
* Select an additional field.
*
* @param $field The field to select
* @param $alias The alias of that field
* @param $field String The field to select (escaped SQL statement)
* @param $alias String The alias of that field (escaped SQL statement).
* Defaults to the unquoted column name of the $field parameter.
* @return SQLQuery
*/
public function selectField($field, $alias = null) {
@ -220,6 +221,10 @@ class SQLQuery {
/**
* Return the SQL expression for the given field alias.
* Returns null if the given alias doesn't exist.
* See {@link selectField()} for details on alias generation.
*
* @param String $field
* @return String
*/
public function expressionForField($field) {
return isset($this->select[$field]) ? $this->select[$field] : null;
@ -228,11 +233,9 @@ class SQLQuery {
/**
* Set table for the SELECT clause.
*
* <code>
* $query->setFrom("MyTable"); // SELECT * FROM MyTable
* </code>
* @example $query->setFrom("MyTable"); // SELECT * FROM MyTable
*
* @param string|array $from
* @param string|array $from Escaped SQL statement, usually an unquoted table name
* @return SQLQuery
*/
public function setFrom($from) {
@ -243,11 +246,9 @@ class SQLQuery {
/**
* Add a table to the SELECT clause.
*
* <code>
* $query->addFrom("MyTable"); // SELECT * FROM MyTable
* </code>
* @example $query->addFrom("MyTable"); // SELECT * FROM MyTable
*
* @param string|array $from
* @param string|array $from Escaped SQL statement, usually an unquoted table name
* @return SQLQuery
*/
public function addFrom($from) {
@ -268,8 +269,8 @@ class SQLQuery {
/**
* Add a LEFT JOIN criteria to the FROM clause.
*
* @param string $table Table name (unquoted)
* @param string $onPredicate The "ON" SQL fragment in a "LEFT JOIN ... AS ... ON ..." statement.
* @param string $table Unquoted table name
* @param string $onPredicate The "ON" SQL fragment in a "LEFT JOIN ... AS ... ON ..." statement,
* Needs to be valid (quoted) SQL.
* @param string $tableAlias Optional alias which makes it easier to identify and replace joins later on
* @return SQLQuery
@ -288,7 +289,7 @@ class SQLQuery {
/**
* Add an INNER JOIN criteria to the FROM clause.
*
* @param string $table Table name (unquoted)
* @param string $table Unquoted table name
* @param string $onPredicate The "ON" SQL fragment in an "INNER JOIN ... AS ... ON ..." statement.
* Needs to be valid (quoted) SQL.
* @param string $tableAlias Optional alias which makes it easier to identify and replace joins later on
@ -309,7 +310,7 @@ class SQLQuery {
* Add an additional filter (part of the ON clause) on a join.
*
* @param string $table Table to join on from the original join
* @param string $filter The "ON" SQL fragment
* @param string $filter The "ON" SQL fragment (escaped)
* @return SQLQuery
*/
public function addFilterToJoin($table, $filter) {
@ -321,7 +322,7 @@ class SQLQuery {
* Set the filter (part of the ON clause) on a join.
*
* @param string $table Table to join on from the original join
* @param string $filter The "ON" SQL fragment
* @param string $filter The "ON" SQL fragment (escaped)
* @return SQLQuery
*/
public function setJoinFilter($table, $filter) {
@ -331,6 +332,8 @@ class SQLQuery {
/**
* Returns true if we are already joining to the given table alias
*
* @return boolean
*/
public function isJoinedTo($tableAlias) {
return isset($this->from[$tableAlias]);
@ -338,6 +341,8 @@ class SQLQuery {
/**
* Return a list of tables that this query is selecting from.
*
* @return array Unquoted table names
*/
public function queriedTables() {
$tables = array();
@ -416,7 +421,8 @@ class SQLQuery {
* Pass LIMIT clause either as SQL snippet or in array format.
* Internally, limit will always be stored as a map containing the keys 'start' and 'limit'
*
* @param string|array $limit
* @param int|string|array $limit If passed as a string or array, assumes SQL escaped data.
* @param int $offset
* @return SQLQuery This instance
*/
public function setLimit($limit, $offset = 0) {
@ -453,7 +459,7 @@ class SQLQuery {
* @example $sql->orderby("Column", "DESC");
* @example $sql->orderby(array("Column" => "ASC", "ColumnTwo" => "DESC"));
*
* @param string|array $orderby Clauses to add
* @param string|array $orderby Clauses to add (escaped SQL statement)
* @param string $dir Sort direction, ASC or DESC
*
* @return SQLQuery
@ -472,7 +478,7 @@ class SQLQuery {
* @example $sql->orderby("Column", "DESC");
* @example $sql->orderby(array("Column" => "ASC", "ColumnTwo" => "DESC"));
*
* @param string|array $orderby Clauses to add
* @param string|array $orderby Clauses to add (escaped SQL statements)
* @param string $dir Sort direction, ASC or DESC
*
* @return SQLQuery
@ -546,7 +552,9 @@ class SQLQuery {
/**
* Extract the direction part of a single-column order by clause.
*
* Return a 2 element array: array($column, $direction)
* @param String
* @param String
* @return Array A two element array: array($column, $direction)
*/
private function getDirectionFromString($value, $defaultDirection = null) {
if(preg_match('/^(.*)(asc|desc)$/i', $value, $matches)) {
@ -611,7 +619,7 @@ class SQLQuery {
/**
* Set a GROUP BY clause.
*
* @param string|array $groupby
* @param string|array $groupby Escaped SQL statement
* @return SQLQuery
*/
public function setGroupBy($groupby) {
@ -622,7 +630,7 @@ class SQLQuery {
/**
* Add a GROUP BY clause.
*
* @param string|array $groupby
* @param string|array $groupby Escaped SQL statement
* @return SQLQuery
*/
public function addGroupBy($groupby) {
@ -654,7 +662,7 @@ class SQLQuery {
/**
* Add a HAVING clause
*
* @param string|array $having
* @param string|array $having Escaped SQL statement
* @return SQLQuery
*/
public function addHaving($having) {
@ -685,7 +693,7 @@ class SQLQuery {
* $query->where(array("Column = 'Value'", "Column != 'Value'"));
* </code>
*
* @param string|array $where Predicate(s) to set
* @param string|array $where Predicate(s) to set, as escaped SQL statements.
* @return SQLQuery
*/
public function setWhere($where) {
@ -712,7 +720,7 @@ class SQLQuery {
* $query->where(array("Column = 'Value'", "Column != 'Value'"));
* </code>
*
* @param string|array $where Predicate(s) to set
* @param string|array $where Predicate(s) to set, as escaped SQL statements.
* @return SQLQuery
*/
public function addWhere($where) {
@ -731,7 +739,7 @@ class SQLQuery {
}
/**
*
* @param String|array $filters Predicate(s) to set, as escaped SQL statements.
*/
function whereAny($filters) {
if(is_string($filters)) $filters = func_get_args();
@ -755,8 +763,9 @@ class SQLQuery {
/**
* Swap the use of one table with another.
* @param string $old Name of the old table.
* @param string $new Name of the new table.
*
* @param string $old Name of the old table (unquoted, escaped)
* @param string $new Name of the new table (unquoted, escaped)
*/
function renameTable($old, $new) {
$this->replaceText("`$old`", "`$new`");
@ -765,8 +774,9 @@ class SQLQuery {
/**
* Swap some text in the SQL query with another.
* @param string $old The old text.
* @param string $new The new text.
*
* @param string $old The old text (escaped)
* @param string $new The new text (escaped)
*/
function replaceText($old, $new) {
$this->replacementsOld[] = $old;
@ -837,8 +847,8 @@ class SQLQuery {
else if(sizeof($join['filter']) == 1) $filter = $join['filter'][0];
else $filter = "(" . implode(") AND (", $join['filter']) . ")";
$aliasClause = ($alias != $join['table']) ? " AS \"$alias\"" : "";
$this->from[$alias] = strtoupper($join['type']) . " JOIN \"{$join['table']}\"$aliasClause ON $filter";
$aliasClause = ($alias != $join['table']) ? " AS \"" . Convert::raw2sql($alias) . "\"" : "";
$this->from[$alias] = strtoupper($join['type']) . " JOIN \"{" . Convert::raw2sql($join['table']) . "}\"$aliasClause ON $filter";
}
}
@ -959,9 +969,11 @@ class SQLQuery {
/**
* Return the number of rows in this query if the limit were removed. Useful in paged data sets.
* @return int
*
* TODO Respect HAVING and GROUPBY, which can affect the result-count
* @todo Respect HAVING and GROUPBY, which can affect the result-count
*
* @param String $column Quoted, escaped column name
* @return int
*/
function count( $column = null) {
// Choose a default column
@ -995,7 +1007,7 @@ class SQLQuery {
/**
* Return a new SQLQuery that calls the given aggregate functions on this data.
* @param $column An aggregate expression, such as 'MAX("Balance")', or a set of them.
* @param $column An aggregate expression, such as 'MAX("Balance")', or a set of them (as an escaped SQL statement)
*/
function aggregate($column) {
if($this->groupby || $this->limit) {