Restrict allowed additional attributes to prevent self XSS

Make Drupal\ckeditor5\HTMLRestrictionsUtilities disallow on* attributes.

I think there's actually two parts:

1. Disallow configuring sourceEditing to allow on*, style or any of the tags in \Drupal\editor\EditorXssFilter\Standard. This will require just a validation constraint, nothing needs to change in HTMLRestrictionsUtilities AFAICT.
2. Disallow it for the "arbitrary HTML" scenario by changing
   

   ckeditor5_htmlSupport:
     ckeditor5:
   …
       config:
         htmlSupport:
           allow:
             -
               name:
                 regexp:
                   pattern: /.*/
               attributes: true
               classes: true
     drupal:
       …
   

   to disallow style and on* and …

Or … what is more likely … I am missing something. Because what I'm saying is very different from what's the proposed resolution in the IS :P

CKEditor 4 full HTML text formats allow using both style and on* attributes. For that reason full HTML text formats are documented as unsafe in various different places including https://www.drupal.org/docs/core-modules-and-themes/core-modules/filter-....

Ah, good point!

But those special tags would still get stripped by the text editor XSS filtering… Did a deep dive, and confirmed that @lauriii's comment in #4 is accurate! See \editor_filter_xss(), which contains this:

  // If there is no filter preventing XSS attacks in the text format being used,
  // then no text editor XSS filtering is needed either. (Because then the
  // editing user can already be attacked by merely viewing the content.)
  // e.g.: an admin user creates content in Full HTML and then edits it, no text
  // format switching happens; in this case, no text editor XSS filtering is
  // desirable, because it would strip style attributes, amongst others.
  $current_filter_types = $format->getFilterTypes();
  if (!in_array(FilterInterface::TYPE_HTML_RESTRICTOR, $current_filter_types, TRUE)) {
    if ($original_format === NULL) {
      return FALSE;
    }
…

Perfect!

This should wait for #3228334: Refactor HTMLRestrictionsUtilities to a HtmlRestrictions value object to finish; that will make this simpler.

#3228334: Refactor HTMLRestrictionsUtilities to a HtmlRestrictions value object is in!

Re-reading this, my analysis in #2 is kinda wrong. The second point is indeed not necessary like @lauriii pointed out in #4. Because CKEditor 5 does not able to restrict things, because it only is able to allow extra things! I see I already realized this in #5 :P

So what we need is something like this.

Related: #3260857: Expand SourceEditingRedundantTagsConstraintValidator to also check attributes and attribute values.

Tests added in https://git.drupalcode.org/project/drupal/-/merge_requests/1933/diffs?co...

Logic also implemented. Tests should pass. Ready for review! 😊

Having just reviewed #3261600: Update to CKEditor5 v32.0.0, I was reminded of #3245950: [upstream] <script> tag support in GHS.

Which makes me wonder if we should make this behave differently than what the issue originally described:

1. on formats with no HTML restrictions (FilterFormat::getHtmlRestrictions() === FALSE), don't do this validation, but instead disallow configuring anything at all — everything is already allowed anyway, so no need to configure explicit additional tags/attributes
2. on formats with HTML restrictions, do perform this validation … but also disallow <script> and <style>. But then where does it stop — perhaps also <iframe> and <embed> should be forbidden? We should allow anything the user wants, unless it is going to be stripped by the filters, which brings me to my next point:
3. Instead of hardcoding things (style and on*), use the restrictions that the text format itself provides! \Drupal\filter\Plugin\Filter\FilterHtml::getHTMLRestrictions() contains:
   

       // The 'style' and 'on*' ('onClick' etc.) attributes are always forbidden,
       // and are removed by Xss::filter().
       // The 'lang', and 'dir' attributes apply to all elements and are always
       // allowed. The list of allowed values for the 'dir' attribute is enforced
       // by self::filterAttributes(). Note that those two attributes are in the
       // short list of globally usable attributes in HTML5. They are always
       // allowed since the correct values of lang and dir may only be known to
       // the content author. Of the other global attributes, they are not usually
       // added by hand to content, and especially the class attribute can have
       // undesired visual effects by allowing content authors to apply any
       // available style, so specific values should be explicitly allowed.
       // @see http://www.w3.org/TR/html5/dom.html#global-attributes
       $restrictions['allowed']['*'] = [
         'style' => FALSE,
         'on*' => FALSE,
         'lang' => TRUE,
         'dir' => ['ltr' => TRUE, 'rtl' => TRUE],
       ];
   

   That is declaring "disallow the style and on* attributes" already! We should probably just interpret that?!

   Automatically interpreting the text format's HTML restrictions would currently result in InvalidArgumentException: "*" is not a valid HTML tag name. in Drupal\ckeditor5\HTMLRestrictions::validateAllowedRestrictionsPhase1() (line 113 of core/modules/ckeditor5/src/HTMLRestrictions.php). though, so I'd like confirmation first that we indeed want to do this. Because it requires removing this:

       // @todo Validate attributes allowed or forbidden on all elements
       //   https://www.drupal.org/project/ckeditor5/issues/3231334.
       if (isset($allowed['*'])) {
         unset($allowed['*']);
       }
   

   and hence we'd need to do #3231334: Global attributes (<* lang> and <* dir="ltr rtl">): validation + support (fix data loss) first.

+1. I'll start that.

#3231334: Global attributes (<* lang> and <* dir="ltr rtl">): validation + support (fix data loss) landed! Time to update this. Let's first rebase.

#14 has now been implemented! 🤓👍 Ready for review again!

And that was the very final piece, a @todo referring to #3260869: Resolve mismatch between <$block> interpretation by CKEditor 5 and Drupal, also is done — because #3260869 landed a while ago :)

Added a comment for a small detail, other than that it makes sense, test look good, and works in manual testing too.

Commit d75fb448 as patch, should apply to all 3 branches.

(The 9.3.x failures are expected: they happen because #3231334: Global attributes (<* lang> and <* dir="ltr rtl">): validation + support (fix data loss) has not yet been cherry-picked to 9.3.x.)

Committed to 10.0.x and cherry-picked to 9.4.x and 9.3.x (9.3 backport since CKEditor5 is experimental and making a module more secure oughta go anywhere it can).