Merge pull request #11201 from creative-commoners/pulls/5/fix-wysiwyg-sanitisation

Fix wysiwyg sanitisation
This commit is contained in:
Steve Boyd 2024-04-19 11:02:44 +12:00 committed by GitHub
commit c2199ff88b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 98 additions and 19 deletions

View File

@ -138,7 +138,8 @@ class HTMLEditorField extends TextareaField
// Sanitise if requested
$htmlValue = HTMLValue::create($this->Value());
if (HTMLEditorField::config()->sanitise_server_side) {
$santiser = HTMLEditorSanitiser::create(HTMLEditorConfig::get_active());
$config = $this->getEditorConfig();
$santiser = HTMLEditorSanitiser::create($config);
$santiser->sanitise($htmlValue);
}

View File

@ -287,10 +287,6 @@ class HTMLEditorSanitiser
*/
public function sanitise(HTMLValue $html)
{
if (!$this->elements && !$this->elementPatterns) {
return;
}
$linkRelValue = $this->config()->get('link_rel_value');
$doc = $html->getDocument();

View File

@ -250,13 +250,11 @@ class TinyMCEConfig extends HTMLEditorConfig implements i18nEntityProvider
private static $image_size_presets = [ ];
/**
* TinyMCE JS settings
* Default TinyMCE JS options which apply to all new configurations.
*
* @link https://www.tiny.cloud/docs/tinymce/6/tinydrive-getting-started/#configure-the-required-tinymce-options
*
* @var array
*/
protected $settings = [
private static array $default_options = [
'fix_list_elements' => true, // https://www.tiny.cloud/docs/tinymce/6/content-filtering/#fix_list_elements
'formats' => [
'alignleft' => [
@ -311,8 +309,24 @@ class TinyMCEConfig extends HTMLEditorConfig implements i18nEntityProvider
'promotion' => false,
'upload_folder_id' => null, // Set folder ID for insert media dialog
'link_default_target' => '_blank', // https://www.tiny.cloud/docs/tinymce/6/autolink/#example-using-link_default_target
// Default set of valid_elements which apply for all new configurations
'valid_elements' => "@[id|class|style|title],a[id|rel|rev|dir|tabindex|accesskey|type|name|href|target|title"
. "|class],-strong/-b[class],-em/-i[class],-strike[class],-u[class],#p[id|dir|class|align|style],-ol[class],"
. "-ul[class],-li[class],br,img[id|dir|longdesc|usemap|class|src|border|alt=|title|hspace|vspace|width|height|align|name|data*],"
. "-sub[class],-sup[class],-blockquote[dir|class],-cite[dir|class|id|title],"
. "-table[cellspacing|cellpadding|width|height|class|align|summary|dir|id|style],"
. "-tr[id|dir|class|rowspan|width|height|align|valign|bgcolor|background|bordercolor|style],"
. "tbody[id|class|style],thead[id|class|style],tfoot[id|class|style],"
. "#td[id|dir|class|colspan|rowspan|width|height|align|valign|scope|style],"
. "-th[id|dir|class|colspan|rowspan|width|height|align|valign|scope|style],caption[id|dir|class],"
. "-div[id|dir|class|align|style],-span[class|align|style],-pre[class|align],address[class|align],"
. "-h1[id|dir|class|align|style],-h2[id|dir|class|align|style],-h3[id|dir|class|align|style],"
. "-h4[id|dir|class|align|style],-h5[id|dir|class|align|style],-h6[id|dir|class|align|style],hr[class],"
. "dd[id|class|title|dir],dl[id|class|title|dir],dt[id|class|title|dir],"
];
protected $settings = [];
/**
* Holder list of enabled plugins
*
@ -337,6 +351,11 @@ class TinyMCEConfig extends HTMLEditorConfig implements i18nEntityProvider
*/
protected $theme = 'silver';
public function __construct()
{
$this->settings = static::config()->get('default_options');
}
/**
* Get the theme
*

View File

@ -12,6 +12,7 @@ use SilverStripe\Control\Director;
use SilverStripe\Core\Config\Config;
use SilverStripe\Dev\CSSContentParser;
use SilverStripe\Dev\FunctionalTest;
use SilverStripe\Forms\HTMLEditor\HTMLEditorConfig;
use SilverStripe\Forms\HTMLEditor\HTMLEditorField;
use SilverStripe\Forms\HTMLEditor\TinyMCEConfig;
use SilverStripe\Forms\HTMLReadonlyField;
@ -278,4 +279,41 @@ EOS
$this->assertEquals("auto", $data_config->height, 'Config height is not set');
$this->assertEquals("60px", $data_config->row_height, 'Config row_height is not set');
}
public function testFieldConfigSanitization()
{
$obj = TestObject::create();
$editor = HTMLEditorField::create('Content');
$defaultValidElements = [
'@[id|class|style|title|data*]',
'a[id|rel|dir|tabindex|accesskey|type|name|href|target|title|class]',
'-strong/-b[class]',
'-em/-i[class]',
'-ol[class]',
'#p[id|dir|class|align|style]',
'-li[class]',
'br',
'-span[class|align|style]',
'-ul[class]',
'-h3[id|dir|class|align|style]',
'-h2[id|dir|class|align|style]',
'hr[class]',
];
$restrictedConfig = HTMLEditorConfig::get('restricted');
$restrictedConfig->setOption('valid_elements', implode(',', $defaultValidElements));
$editor->setEditorConfig($restrictedConfig);
$expectedHtmlString = '<p>standard text</p>Header';
$htmlValue = '<p>standard text</p><table><th><tr><td>Header</td></tr></th><tbody></tbody></table>';
$editor->setValue($htmlValue);
$editor->saveInto($obj);
$this->assertEquals($expectedHtmlString, $obj->Content, 'Table is not removed');
$defaultConfig = HTMLEditorConfig::get('default');
$editor->setEditorConfig($defaultConfig);
$editor->setValue($htmlValue);
$editor->saveInto($obj);
$this->assertEquals($htmlValue, $obj->Content, 'Table is removed');
}
}

View File

@ -11,9 +11,9 @@ use SilverStripe\View\Parsers\HTMLValue;
class HTMLEditorSanitiserTest extends FunctionalTest
{
public function testSanitisation()
public function provideSanitise(): array
{
$tests = [
return [
[
'p,strong',
'<p>Leave Alone</p><div>Strip parent<strong>But keep children</strong> in order</div>',
@ -129,13 +129,20 @@ class HTMLEditorSanitiserTest extends FunctionalTest
'XSS vulnerable attributes starting with on or style are removed via configuration'
],
];
}
$config = HTMLEditorConfig::get('htmleditorsanitisertest');
foreach ($tests as $test) {
list($validElements, $input, $output, $desc) = $test;
$config->setOptions(['valid_elements' => $validElements]);
/**
* @dataProvider provideSanitise
*/
public function testSanitisation(string $validElements, string $input, string $output, string $desc): void
{
foreach (['valid_elements', 'extended_valid_elements'] as $configType) {
$config = HTMLEditorConfig::get('htmleditorsanitisertest_' . $configType);
$config->setOptions([$configType => $validElements]);
// Remove default valid elements if we're testing extended valid elements
if ($configType !== 'valid_elements') {
$config->setOptions(['valid_elements' => '']);
}
$sanitiser = new HtmlEditorSanitiser($config);
$value = 'noopener noreferrer';
@ -144,12 +151,30 @@ class HTMLEditorSanitiserTest extends FunctionalTest
} elseif (strpos($desc ?? '', 'link_rel_value is null') !== false) {
$value = null;
}
Config::inst()->set(HTMLEditorSanitiser::class, 'link_rel_value', $value);
HTMLEditorSanitiser::config()->set('link_rel_value', $value);
$htmlValue = HTMLValue::create($input);
$sanitiser->sanitise($htmlValue);
$this->assertEquals($output, $htmlValue->getContent(), $desc);
$this->assertEquals($output, $htmlValue->getContent(), "{$desc} - using config type: {$configType}");
}
}
/**
* Ensure that when there are no valid elements at all for a configuration set,
* nothing is allowed.
*/
public function testSanitiseNoValidElements(): void
{
$config = HTMLEditorConfig::get('htmleditorsanitisertest');
$config->setOptions(['valid_elements' => '']);
$config->setOptions(['extended_valid_elements' => '']);
$sanitiser = new HtmlEditorSanitiser($config);
$htmlValue = HTMLValue::create('<p>standard text</p><table><tbody><tr><th><a href="some-link">text</a></th></tr><tr><td>Header</td></tr></tbody></table>');
$sanitiser->sanitise($htmlValue);
$this->assertEquals('standard texttextHeader', $htmlValue->getContent());
}
}