tonyair
/
silverstripe-framework
mirror of https://github.com/silverstripe/silverstripe-framework
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge branch '4.3' into 4

This commit is contained in:
Aaron Carlino 2019-02-19 12:21:24 +13:00
parent 5a4a36a79e 70fc2d6f1d
commit 09c539e1c3
11 changed files with 181 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
docs/en/04_Changelogs/4.3.1.md Normal file
View File

@ -0,0 +1,9 @@
# 4.3.1
<!--- Changes below this line will be automatically regenerated -->
## Change Log
### Security
* 2018-12-18 [c0338d191](https://github.com/silverstripe/silverstripe-framework/commit/c0338d191d8be0000ddb16b74832ed8e05ba7ff5) Fix potential SQL vulnerability in non-scalar value hyrdation (Maxime Rainville) - See [ss-2018-021](https://www.silverstripe.org/download/security-releases/ss-2018-021)

27
src/ORM/DataObject.php
View File

@ -1455,6 +1455,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
* @param string $baseTable Base table
* @param string $now Timestamp to use for the current time
* @param bool $isNewRecord If this is a new record
* @throws InvalidArgumentException
*/
protected function writeManipulation($baseTable, $now, $isNewRecord)
{
@ -1473,6 +1474,20 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
$manipulation[$baseTable]['command'] = 'update';
}
// Make sure none of our field assignment are arrays
foreach ($manipulation as $tableManipulation) {
if (!isset($tableManipulation['fields'])) {
continue;
}
foreach ($tableManipulation['fields'] as $fieldValue) {
if (is_array($fieldValue)) {
throw new InvalidArgumentException(
'DataObject::writeManipulation: parameterised field assignments are disallowed'
);
}
}
}
// Perform the manipulation
DB::manipulate($manipulation);
}
@ -2649,6 +2664,18 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
throw new InvalidArgumentException('DataObject::setField: passed an object that is not a DBField');
}
if (!empty($val) && !is_scalar($val)) {
$dbField = $this->dbObject($fieldName);
if ($dbField && $dbField->scalarValueOnly()) {
throw new InvalidArgumentException(
sprintf(
'DataObject::setField: %s only accepts scalars',
$fieldName
)
);
}
}
// if a field is not existing or has strictly changed
if (!array_key_exists($fieldName, $this->original) || $this->original[$fieldName] !== $val) {
// TODO Add check for php-level defaults which are not set in the db

5
src/ORM/FieldType/DBComposite.php
View File

@ -339,4 +339,9 @@ abstract class DBComposite extends DBField
];
}
}
public function scalarValueOnly()
{
return false;
}
}

23
src/ORM/FieldType/DBField.php
View File

@ -335,18 +335,16 @@ abstract class DBField extends ViewableData implements DBIndexable
* will be escaped automatically by the prepared query processor, so it
* should not be escaped or quoted at all.
*
* The field values could also be in paramaterised format, such as
* array('MAX(?,?)' => array(42, 69)), allowing the use of raw SQL values such as
* array('NOW()' => array()).
*
* @see SQLWriteExpression::addAssignments for syntax examples
*
* @param $value mixed The value to check
* @return mixed The raw value, or escaped parameterised details
*/
public function prepValueForDB($value)
{
if ($value === null || $value === "" || $value === false) {
if ($value === null ||
$value === "" ||
$value === false ||
($this->scalarValueOnly() && !is_scalar($value))
) {
return null;
} else {
return $value;
@ -653,4 +651,15 @@ DBG;
}
return null;
}
/**
* Whether or not this DBField only accepts scalar values.
*
* Composite DBFields can override this method and return `false` so they can accept arrays of values.
* @return boolean
*/
public function scalarValueOnly()
{
return true;
}
}

24
src/ORM/FieldType/DBForeignKey.php
View File

@ -43,6 +43,13 @@ class DBForeignKey extends DBInt
private static $default_search_filter_class = 'ExactMatchFilter';
/**
* Cache for multiple subsequent calls to scaffold form fields with the same foreign key object
*
* @var array
*/
protected static $foreignListCache = [];
public function __construct($name, $object = null)
{
$this->object = $object;
@ -74,8 +81,21 @@ class DBForeignKey extends DBInt
// Don't scaffold a dropdown for large tables, as making the list concrete
// might exceed the available PHP memory in creating too many DataObject instances
$threshold = self::config()->get('dropdown_field_threshold');
if ($list->count() < $threshold) {
$field = new DropdownField($this->name, $title, $list->map('ID', $titleField));
// Add the count of the list to a cache for subsequent calls
if (!isset(static::$foreignListCache[$hasOneClass])) {
static::$foreignListCache[$hasOneClass] = [
'count' => $list->count(),
];
}
if (static::$foreignListCache[$hasOneClass]['count'] < $threshold) {
// Add the mapped list for the cache
if (!isset(static::$foreignListCache[$hasOneClass]['map'])) {
static::$foreignListCache[$hasOneClass]['map'] = $list->map('ID', $titleField);
}
$field = new DropdownField($this->name, $title, static::$foreignListCache[$hasOneClass]['map']);
$field->setEmptyString(' ');
} else {
$field = new NumericField($this->name, $title);

2
src/ORM/FieldType/DBString.php
View File

@ -94,7 +94,7 @@ abstract class DBString extends DBField
public function prepValueForDB($value)
{
// Cast non-empty value
if (strlen($value)) {
if (is_scalar($value) && strlen($value)) {
return (string)$value;
}

14
src/ORM/ManyManyList.php
View File

@ -300,6 +300,20 @@ class ManyManyList extends RelationList
$manipulation[$this->joinTable]['fields'][$this->localKey] = $itemID;
$manipulation[$this->joinTable]['fields'][$this->foreignKey] = $foreignID;
// Make sure none of our field assignments are arrays
foreach ($manipulation as $tableManipulation) {
if (!isset($tableManipulation['fields'])) {
continue;
}
foreach ($tableManipulation['fields'] as $fieldValue) {
if (is_array($fieldValue)) {
throw new InvalidArgumentException(
'ManyManyList::add: parameterised field assignments are disallowed'
);
}
}
}
DB::manipulate($manipulation);
}
}

63
tests/php/ORM/DBFieldTest.php
View File

@ -2,17 +2,32 @@
namespace SilverStripe\ORM\Tests;
use SilverStripe\Assets\Image;
use SilverStripe\ORM\FieldType\DBBigInt;
use SilverStripe\ORM\FieldType\DBBoolean;
use SilverStripe\ORM\FieldType\DBCurrency;
use SilverStripe\ORM\FieldType\DBDate;
use SilverStripe\ORM\FieldType\DBDatetime;
use SilverStripe\ORM\FieldType\DBDecimal;
use SilverStripe\ORM\FieldType\DBDouble;
use SilverStripe\ORM\FieldType\DBEnum;
use SilverStripe\ORM\FieldType\DBFloat;
use SilverStripe\ORM\FieldType\DBForeignKey;
use SilverStripe\ORM\FieldType\DBHTMLText;
use SilverStripe\ORM\FieldType\DBHTMLVarchar;
use SilverStripe\ORM\FieldType\DBInt;
use SilverStripe\ORM\FieldType\DBLocale;
use SilverStripe\ORM\FieldType\DBMoney;
use SilverStripe\ORM\FieldType\DBMultiEnum;
use SilverStripe\ORM\FieldType\DBPercentage;
use SilverStripe\ORM\FieldType\DBPolymorphicForeignKey;
use SilverStripe\ORM\FieldType\DBPrimaryKey;
use SilverStripe\ORM\FieldType\DBString;
use SilverStripe\ORM\FieldType\DBTime;
use SilverStripe\ORM\FieldType\DBVarchar;
use SilverStripe\ORM\FieldType\DBText;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\ORM\FieldType\DBYear;
/**
* Tests for DBField objects.
@ -189,6 +204,54 @@ class DBFieldTest extends SapphireTest
$this->assertEquals(PHP_INT_MAX, $bigInt->getValue());
}
/**
* @dataProvider dataProviderPrepValueForDBArrayValue
*/
public function testPrepValueForDBArrayValue($dbFieldName, $scalarValueOnly, $extraArgs = [])
{
$reflection = new \ReflectionClass($dbFieldName);
/**
* @var DBField
*/
$dbField = $reflection->newInstanceArgs($extraArgs);
$dbField->setName('SomeField');
$payload = ['GREATEST(0,?)' => '2'];
$preparedValue = $dbField->prepValueForDB($payload);
$this->assertTrue(
!$scalarValueOnly || !is_array($preparedValue),
'`prepValueForDB` can not return an array if scalarValueOnly is true'
);
$this->assertEquals($scalarValueOnly, $dbField->scalarValueOnly());
}
public function dataProviderPrepValueForDBArrayValue()
{
return [
[DBBigInt::class, true],
[DBBoolean::class, true],
[DBCurrency::class, true],
[DBDate::class, true],
[DBDatetime::class, true],
[DBDecimal::class, true],
[DBDouble::class, true],
[DBEnum::class, true],
[DBFloat::class, true],
[DBForeignKey::class, true, ['SomeField']],
[DBHTMLText::class, true],
[DBHTMLVarchar::class, true],
[DBInt::class, true],
[DBLocale::class, true],
[DBMoney::class, false],
[DBMultiEnum::class, true, ['SomeField', ['One', 'Two', 'Three']]],
[DBPercentage::class, true],
[DBPolymorphicForeignKey::class, false, ['SomeField']],
[DBText::class, true],
[DBTime::class, true],
[DBVarchar::class, true],
[DBYear::class, true],
];
}
public function testExists()
{
$varcharField = new DBVarchar("testfield");

18
tests/php/ORM/DataObjectTest.php
View File

@ -17,7 +17,9 @@ use SilverStripe\ORM\FieldType\DBField;
use SilverStripe\ORM\FieldType\DBPolymorphicForeignKey;
use SilverStripe\ORM\FieldType\DBVarchar;
use SilverStripe\ORM\ManyManyList;
use SilverStripe\ORM\Tests\DataObjectTest\Company;
use SilverStripe\ORM\Tests\DataObjectTest\Player;
use SilverStripe\ORM\Tests\DataObjectTest\Team;
use SilverStripe\View\ViewableData;
use stdClass;
@ -2318,4 +2320,20 @@ class DataObjectTest extends SapphireTest
['"DataObjectTest_TeamComment"."Name"' => 'does not exists']
));
}
public function testSetFieldWithArrayOnScalarOnlyField()
{
$this->expectException(InvalidArgumentException::class);
$do = Company::singleton();
$do->FoundationYear = '1984';
$do->FoundationYear = array('Amount' => 123, 'Currency' => 'CAD');
$this->assertEmpty($do->FoundationYear);
}
public function testSetFieldWithArrayOnCompositeField()
{
$do = Company::singleton();
$do->SalaryCap = array('Amount' => 123456, 'Currency' => 'CAD');
$this->assertNotEmpty($do->SalaryCap);
}
}

4
tests/php/ORM/DataObjectTest/Company.php
View File

@ -10,7 +10,9 @@ class Company extends DataObject implements TestOnly
private static $table_name = 'DataObjectTest_Company';
private static $db = [
'Name' => 'Varchar'
'Name' => 'Varchar',
'MarketValue' => 'Money',
'FoundationYear' => 'Year'
];
private static $has_one = [

4
tests/php/ORM/DataObjectTest/Team.php
View File

@ -10,6 +10,8 @@ use SilverStripe\ORM\ManyManyList;
/**
* @property string Title
* @property string DatabaseField
* @property array SalaryCap
* @property string FoundationYear
* @method Player Captain()
* @method Player Founder()
* @method Player HasOneRelationship()
@ -27,7 +29,7 @@ class Team extends DataObject implements TestOnly
private static $db = array(
'Title' => 'Varchar',
'DatabaseField' => 'HTMLVarchar'
'DatabaseField' => 'HTMLVarchar',
);
private static $has_one = array(