Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/Assets/Uploader.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use Facades\Statamic\Imaging\ImageValidator;
use Statamic\Facades\Glide;
use Statamic\Support\Str;
use Statamic\Support\Svg;
use Symfony\Component\HttpFoundation\File\UploadedFile;

Expand Down Expand Up @@ -58,7 +57,7 @@ private function write($sourcePath, $destinationPath)
{
$stream = fopen($sourcePath, 'r');

if (config('statamic.assets.svg_sanitization_on_upload', true) && Str::endsWith($destinationPath, '.svg')) {
if (config('statamic.assets.svg_sanitization_on_upload', true) && trim(strtolower(pathinfo($destinationPath, PATHINFO_EXTENSION))) === 'svg') {
$stream = Svg::sanitize(stream_get_contents($stream));
}

Expand Down
7 changes: 0 additions & 7 deletions src/Modifiers/CoreModifiers.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
use Statamic\Support\Arr;
use Statamic\Support\Dumper;
use Statamic\Support\Html;
use Statamic\Support\MethodDenylist;
use Statamic\Support\Str;
use Statamic\Support\Traits\ChecksDumpability;
use Statamic\View\Antlers\Language\Runtime\GlobalRuntimeState;
Expand Down Expand Up @@ -904,12 +903,6 @@ public function get($value, $params)
return Arr::get($array, $var);
}

// Finally, try to call a method on the object
$method = Str::slug($var);
if (method_exists($item, $method) && ! MethodDenylist::blocks($method)) {
return $item->$method();
}

// If after all is said and done, there's still nothing, just show the original value.
return $value;
}
Expand Down
8 changes: 5 additions & 3 deletions src/View/Antlers/Language/Runtime/PathDataManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -863,9 +863,11 @@ private function reduceVar($path, $processorData = [])
}

if (is_object($this->reducedVar) && method_exists($this->reducedVar, $method = Str::camel($varPath)) && (new \ReflectionMethod($this->reducedVar, $method))->isPublic()) {
if (MethodDenylist::blocks($method)) {
// The method name derives from user-influenceable data, so never
// dispatch to methods that mutate or destroy data. Resolve to null.
// The method name derives from user-influenceable data, so never dispatch to
// methods that mutate or destroy data. Writing `{{ object.method }}` without
// parentheses calls the method just like `{{ object:method() }}` does, so both
// forms honor the `statamic.antlers.allowMethodsInContent` setting.
if (MethodDenylist::blocks($method) || (GlobalRuntimeState::$isEvaluatingUserData && ! GlobalRuntimeState::$allowMethodsInContent)) {
$this->reducedVar = null;
$this->didFind = false;
$this->doBreak = true;
Expand Down
7 changes: 7 additions & 0 deletions tests/Antlers/Fixtures/MethodClasses/CallCounter.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ public function increment()
return $this;
}

public function incrementTwice()
{
$this->count += 2;

return $this;
}

public function __toString(): string
{
return 'Count: '.$this->count;
Expand Down
54 changes: 54 additions & 0 deletions tests/Antlers/Runtime/MethodCallTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,60 @@ public function test_method_calls_still_work_in_templates()
], false, true));
}

public function test_implicit_method_calls_blocked_in_user_content()
{
$counter = new CallCounter();

// Resolving a variable path to a zero-argument method is still a method
// call, so it honors the same setting as the explicit `:method()` syntax.
$this->assertSame('', $this->renderUserContent('{{ counter.increment }}', ['counter' => $counter]));
$this->assertSame('', $this->renderUserContent('{{ counter:increment }}', ['counter' => $counter]));

// Str::camel() maps a snake_case path onto a camelCase method.
$this->assertSame('', $this->renderUserContent('{{ counter.increment_twice }}', ['counter' => $counter]));

$this->assertSame('Count: 0', (string) $counter);
}

public function test_implicit_method_calls_allowed_in_user_content_when_configured()
{
GlobalRuntimeState::$allowMethodsInContent = true;

$object = new StringLengthObject('Hello');

$this->assertSame('5', $this->renderUserContent('{{ object.length }}', ['object' => $object]));

GlobalRuntimeState::$allowMethodsInContent = false;
}

public function test_implicit_method_calls_still_work_in_templates()
{
$object = new StringLengthObject('Hello');

$this->assertSame('5', $this->renderString('{{ object.length }}', [
'object' => $object,
], false, true));

$this->assertSame('5', $this->renderString('{{ object:length }}', [
'object' => $object,
], false, true));
}

private function renderUserContent($content, $data)
{
$textFieldtype = new Text();
$field = new Field('text_field', [
'type' => 'text',
'antlers' => true,
]);

$textFieldtype->setField($field);

return $this->renderString('{{ text_field }}', array_merge($data, [
'text_field' => new Value($content, 'text_field', $textFieldtype),
]), false, true);
}

public function test_nested_value_does_not_reset_user_data_flag()
{
$textFieldtype = new Text();
Expand Down
45 changes: 44 additions & 1 deletion tests/Assets/AssetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,11 @@
use Symfony\Component\HttpFoundation\StreamedResponse;
use Tests\PreventSavingStacheItemsToDisk;
use Tests\TestCase;
use Tests\WindowsHelpers;

class AssetTest extends TestCase
{
use PreventSavingStacheItemsToDisk;
use PreventSavingStacheItemsToDisk, WindowsHelpers;

private $container;

Expand Down Expand Up @@ -2089,6 +2090,48 @@ public function it_does_not_sanitizes_svgs_on_upload_when_behaviour_is_disabled(
$this->assertStringContainsString('</script>', $asset->contents());
}

#[Test]
#[DataProvider('unnormalizedSvgExtensionProvider')]
public function it_sanitizes_svgs_on_upload_regardless_of_how_the_extension_is_written($extension)
{
if (trim($extension) !== $extension) {
$this->markTestSkippedInWindows('Windows does not allow filenames with trailing whitespace.');
}

Event::fake();

// Disable filename lowercasing so the uppercase extension actually
// reaches the disk, otherwise it'd be normalized before we could
// prove the sanitization check itself is case insensitive.
config()->set('statamic.assets.lowercase', false);

$asset = (new Asset)->container($this->container)->path($path = "path/to/asset.{$extension}")->syncOriginal();

Facades\AssetContainer::shouldReceive('findByHandle')->with('test_container')->andReturn($this->container);
Storage::disk('test')->assertMissing($path);

$return = $asset->upload(UploadedFile::fake()->createWithContent("asset.{$extension}", '<?xml version="1.0" encoding="UTF-8" standalone="no"?><svg xmlns="http://www.w3.org/2000/svg" width="500" height="500"><script type="text/javascript">alert(`Bad stuff could go in here.`);</script></svg>'));

$this->assertEquals($asset, $return);
Storage::disk('test')->assertExists($path);
$this->assertEquals($path, $asset->path());

// Ensure the inline scripts were stripped out.
$this->assertStringNotContainsString('<script', $asset->contents());
$this->assertStringNotContainsString('Bad stuff could go in here.', $asset->contents());
$this->assertStringNotContainsString('</script>', $asset->contents());
}

public static function unnormalizedSvgExtensionProvider()
{
return [
'uppercase' => ['SVG'],
'mixed case' => ['Svg'],
'trailing whitespace' => ['svg '],
'uppercase with trailing whitespace' => ['SVG '],
];
}

public static function nonGlideableFileExtensionsProvider()
{
return [
Expand Down
45 changes: 44 additions & 1 deletion tests/Feature/Fieldtypes/FilesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@
use Statamic\Fieldtypes\Files;
use Tests\PreventSavingStacheItemsToDisk;
use Tests\TestCase;
use Tests\WindowsHelpers;

class FilesTest extends TestCase
{
use PreventSavingStacheItemsToDisk;
use PreventSavingStacheItemsToDisk, WindowsHelpers;

public function setUp(): void
{
Expand Down Expand Up @@ -84,6 +85,48 @@ public function it_uploads_a_file($container, $isImage, $expectedPath, $expected
}
}

#[Test]
#[DataProvider('unnormalizedSvgExtensionProvider')]
public function it_sanitizes_svgs_on_upload_regardless_of_how_the_extension_is_written($extension)
{
if (trim($extension) !== $extension) {
$this->markTestSkippedInWindows('Windows does not allow filenames with trailing whitespace.');
}

Date::setTestNow(Date::createFromTimestamp(1671484636, config('app.timezone')));

$disk = Storage::fake('local');

$file = UploadedFile::fake()->createWithContent("test.{$extension}", '<?xml version="1.0" encoding="UTF-8" standalone="no"?><svg xmlns="http://www.w3.org/2000/svg" width="500" height="500"><script type="text/javascript">alert(`Bad stuff could go in here.`);</script></svg>');

$this
->actingAs(tap(User::make()->makeSuper())->save())
->post('/cp/fieldtypes/files/upload', ['file' => $file])
->assertOk()
->assertJson([
'data' => [
'id' => $path = "1671484636/test.{$extension}",
],
]);

$contents = $disk->get('statamic/file-uploads/'.$path);

// Ensure the inline scripts were stripped out.
$this->assertStringNotContainsString('<script', $contents);
$this->assertStringNotContainsString('Bad stuff could go in here.', $contents);
$this->assertStringNotContainsString('</script>', $contents);
}

public static function unnormalizedSvgExtensionProvider()
{
return [
'uppercase' => ['SVG'],
'mixed case' => ['Svg'],
'trailing whitespace' => ['svg '],
'uppercase with trailing whitespace' => ['SVG '],
];
}

public static function uploadProvider()
{
return [
Expand Down
48 changes: 4 additions & 44 deletions tests/Modifiers/GetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public function it_gets_a_field_value()
}

#[Test]
public function it_returns_falsy_field_values_instead_of_falling_through_to_a_method()
public function it_returns_falsy_field_values_instead_of_the_original_value()
{
$item = new GetTestItem;

Expand All @@ -32,45 +32,20 @@ public function it_returns_falsy_field_values_instead_of_falling_through_to_a_me
}

#[Test]
public function it_dispatches_to_a_non_destructive_accessor_method()
public function it_does_not_dispatch_to_methods()
{
$item = new GetTestItem;

$this->assertEquals('https://example.com', $this->modify($item, 'url'));
}

#[Test]
public function it_does_not_dispatch_to_destructive_methods()
{
$item = new GetTestItem;

$result = $this->modify($item, 'delete');
$this->assertSame($item, $this->modify($item, 'url'));
$this->assertSame($item, $this->modify($item, 'delete'));

$this->assertFalse($item->deleted);
$this->assertSame($item, $result);
}

#[Test]
public function it_does_not_dispatch_to_destructive_methods_case_insensitively()
{
// Str::slug() lowercases the parameter (e.g. "deleteQuietly" => "deletequietly"),
// but method_exists() is case-insensitive, so the denylist must match regardless of case.
$item = new GetTestItem;

$this->modify($item, 'deletequietly');
$this->assertFalse($item->deletedQuietly);

$this->modify($item, 'savequietly');
$this->assertFalse($item->savedQuietly);
}
}

class GetTestItem
{
public $deleted = false;
public $deletedQuietly = false;
public $saved = false;
public $savedQuietly = false;

public function toArray()
{
Expand All @@ -91,19 +66,4 @@ public function delete()
{
$this->deleted = true;
}

public function deleteQuietly()
{
$this->deletedQuietly = true;
}

public function save()
{
$this->saved = true;
}

public function saveQuietly()
{
$this->savedQuietly = true;
}
}
Loading