Skip to content

Commit

Permalink
[5.5] Security fixes (#33858)
Browse files Browse the repository at this point in the history
* fix casing issue with guarded

* block json mass assignment

* formatting

* add boolean

* protect table names and guarded

* Apply fixes from StyleCI (#33772)

* dont allow mass filling with table names

* Apply fixes from StyleCI (#33773)

* [6.x] Verify column names are actual columns when using guarded (#33777)

* verify column names are actual columns when using guarded

* Apply fixes from StyleCI (#33778)

* remove json check

* Apply fixes from StyleCI (#33857)

Co-authored-by: Taylor Otwell <[email protected]>
Co-authored-by: Taylor Otwell <[email protected]>
  • Loading branch information
3 people authored Aug 13, 2020
1 parent a81f23d commit 40579ba
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 7 deletions.
33 changes: 32 additions & 1 deletion src/Illuminate/Database/Eloquent/Concerns/GuardsAttributes.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ trait GuardsAttributes
*/
protected static $unguarded = false;

/**
* The actual columns that exist on the database and can be guarded.
*
* @var array
*/
protected static $guardableColumns = [];

/**
* Get the fillable attributes for the model.
*
Expand Down Expand Up @@ -152,6 +159,7 @@ public function isFillable($key)
}

return empty($this->getFillable()) &&
strpos($key, '.') === false &&
! Str::startsWith($key, '_');
}

Expand All @@ -163,7 +171,30 @@ public function isFillable($key)
*/
public function isGuarded($key)
{
return in_array($key, $this->getGuarded()) || $this->getGuarded() == ['*'];
if (empty($this->getGuarded())) {
return false;
}

return $this->getGuarded() == ['*'] ||
! empty(preg_grep('/^'.preg_quote($key).'$/i', $this->getGuarded())) ||
! $this->isGuardableColumn($key);
}

/**
* Determine if the given column is a valid, guardable column.
*
* @param string $key
* @return bool
*/
protected function isGuardableColumn($key)
{
if (! isset(static::$guardableColumns[get_class($this)])) {
static::$guardableColumns[get_class($this)] = $this->getConnection()
->getSchemaBuilder()
->getColumnListing($this->getTable());
}

return in_array($key, static::$guardableColumns[get_class($this)]);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Database/Eloquent/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ public function qualifyColumn($column)
*/
protected function removeTableFromKey($key)
{
return Str::contains($key, '.') ? last(explode('.', $key)) : $key;
return $key;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Queue/Listener.php
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public function memoryExceeded($memoryLimit)
*/
public function stop()
{
die;
exit;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Support/Collection.php
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ public function dd(...$args)

call_user_func_array([$this, 'dump'], $args);

die(1);
exit(1);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Illuminate/Support/helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ function dd(...$args)
(new Dumper)->dump($x);
}

die(1);
exit(1);
}
}

Expand Down
14 changes: 12 additions & 2 deletions tests/Database/DatabaseEloquentModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -919,11 +919,21 @@ public function testUnderscorePropertiesAreNotFilled()
public function testGuarded()
{
$model = new EloquentModelStub;

EloquentModelStub::setConnectionResolver($resolver = m::mock('Illuminate\Database\ConnectionResolverInterface'));
$resolver->shouldReceive('connection')->andReturn($connection = m::mock(stdClass::class));
$connection->shouldReceive('getSchemaBuilder->getColumnListing')->andReturn(['name', 'age', 'foo']);

$model->guard(['name', 'age']);
$model->fill(['name' => 'foo', 'age' => 'bar', 'foo' => 'bar']);
$this->assertFalse(isset($model->name));
$this->assertFalse(isset($model->age));
$this->assertEquals('bar', $model->foo);
$this->assertSame('bar', $model->foo);

$model = new EloquentModelStub;
$model->guard(['name', 'age']);
$model->fill(['Foo' => 'bar']);
$this->assertFalse(isset($model->Foo));
}

public function testFillableOverridesGuarded()
Expand Down Expand Up @@ -1829,7 +1839,7 @@ public function getDates()
class EloquentModelSaveStub extends Model
{
protected $table = 'save_stub';
protected $guarded = ['id'];
protected $guarded = [];

public function save(array $options = [])
{
Expand Down
27 changes: 27 additions & 0 deletions tests/Integration/Database/EloquentModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,33 @@ public function setUp()
});
}

public function test_cant_update_guarded_attributes_using_different_casing()
{
$model = new TestModel2;

$model->fill(['ID' => 123]);

$this->assertNull($model->ID);
}

public function test_cant_update_guarded_attribute_using_json()
{
$model = new TestModel2;

$model->fill(['id->foo' => 123]);

$this->assertNull($model->id);
}

public function test_cant_mass_fill_attributes_with_table_names_when_using_guarded()
{
$model = new TestModel2;

$model->fill(['foo.bar' => 123]);

$this->assertCount(0, $model->getAttributes());
}

public function test_user_can_update_nullable_date()
{
$user = TestModel1::create([
Expand Down

0 comments on commit 40579ba

Please sign in to comment.