I think comments have been addressed.
Removed patches to focus on the MR.
You don't need to create a new issue in that case, you can update the MR branch or create a new MR.
prudloff → changed the visibility of the branch 3443679-use-elementisempty-to to hidden.
1.2.1 is compatible with Drupal 11 so marking as fixed.
prudloff → created an issue.
Reopening as this is still a problem.
prudloff → created an issue.
I think there could be a valid concern here.
The description of the class does state that this should not be used in HTML contexts but the name of the method could imply that it could be used to transform HTML into harmless plain text.
I have seen this method misused in custom code.
I also would not be surprised if a Google search like "drupal convert HTML to plaintext" points users to this method.
Maybe the description of the method could make it clearer that it is dangerous in HTML contexts?
Rebased and fixed tests.
I think #24 is a valid concern (see 🌱 [policy] Treat username enumerations as security bugs that require Security Advisories Active ).
prudloff → changed the visibility of the branch 2842858-basic-auth-module to hidden.
prudloff → changed the visibility of the branch 10.3.x to hidden.
prudloff → changed the visibility of the branch 10.4.x to hidden.
prudloff → made their first commit to this issue’s fork.
prudloff → changed the visibility of the branch 2846365-regression-user-roles to hidden.
prudloff → changed the visibility of the branch 11.x to hidden.
prudloff → changed the visibility of the branch 9.2.x to hidden.
prudloff → made their first commit to this issue’s fork.
prudloff → changed the visibility of the branch 2940879-dont-automatically-set to hidden.
prudloff → changed the visibility of the branch 11.x to hidden.
prudloff → changed the visibility of the branch 9.2.x to hidden.
prudloff → made their first commit to this issue’s fork.
prudloff → made their first commit to this issue’s fork.
I added and updated the patch from the private issue.
People from the "Fixed by" section of
https://www.drupal.org/sa-core-2024-002 →
should probably be credited.
It is now displayed here: https://github.com/drupal/drupal/security
Oh right, I didn't see that email_login_otp.routing.yml already uses the OTPForm case.
Renaming the class would be a breaking change.
I think it is less disruptive to rename the file to match the class.
prudloff → created an issue.
I tried to rewrite the summary.
prudloff → made their first commit to this issue’s fork.
prudloff → made their first commit to this issue’s fork.
As alexpott said we filter dangerous attributes so most tags that would be dangerous otherwise are not.
However we can't guarantee that we remove dangerous attributes for every possible tag (for example we didn't remove srcdoc attributes on iframe until
🐛
Remove srcdoc attributes in Xss::filter()
Active
).
So in order to keep this simple and not duplicate the list of safe tags, I think we should display the warning when allowing tags that are not in filterAdmin().
This does not mean that any additional tag would be dangerous, but that it could be because our XSS filter might not remove some attribute that could be dangerous on this specific tag.
prudloff → made their first commit to this issue’s fork.
I rebased the latest patch and tests are passing but I did not test manually.
prudloff → made their first commit to this issue’s fork.
prudloff → made their first commit to this issue’s fork.
drumm → credited prudloff → .
prudloff → created an issue.
Are you sure? I can still see the duplicate here: https://git.drupalcode.org/project/email_tfa/-/blob/2.0.x/src/Form/Email...
$form['#cache'] = ['max-age' => 0];
if (!$status) {
return $form;
}
$form['#validate'][] = '::emailTfaLoginFormValidate';
$form['#cache'] = ['max-age' => 0];
prudloff → created an issue. See original summary → .
prudloff → changed the visibility of the branch 3503383-potential-xss-vulnerability to hidden.
prudloff → created an issue.
prudloff → created an issue.
prudloff → created an issue.
@sd123 this is out of the scope of this issue. Feel free to open a separate issue about this (if there isn't one already).
Merged the latest 11.x.
Merged the latest 11.x.
prudloff → changed the visibility of the branch 2844620-finishresponsesubscriber-need-warningerror to hidden.
prudloff → made their first commit to this issue’s fork.
@nicxvan does your MR also solve the problem?
If so, it should have tests (you might be able to reuses the tests from the other MR).
prudloff → changed the visibility of the branch 2118743-twig-debug-output-d10 to hidden.
prudloff → created an issue.
I had to use a trim because the browser can send \r\n line breaks and PHP_EOL can be just \n depending on the OS. So it could leave dangling \r characters.
prudloff → created an issue.
prudloff → created an issue.
prudloff → changed the visibility of the branch 2.0.x to hidden.
I rebased the MR and added a change record: https://www.drupal.org/node/3526457 →
prudloff → changed the visibility of the branch 1587536-js-aggregation-async-defer-attributes-10.1.x to hidden.
Rebased the MR.
this is where aggregate js/css checkboxes are
The aggregate setting is not the same as the gzip/compress setting (which as far as I can tell does not have a UI).
The CR seems to contain some hallucinations:
- The name of the new key is compress, not compression.
- It talks about a setting on /admin/config/development/performance but I see no such setting.
- The name of the update hook is incorrect.
I updated the MR to use a single setting for both gzip and brotli (and future algorithms).
I also added an upgrade path.
We still need a change record I think.
I rebased the latest patch and tests are green.
But I did some manual testing and I think escaping the options is still needed.
For example if I create a role named <b>foo</b> then browse to /admin/config/content/formats/manage/basic_html.
Without the patch:
With the patch:
I think we automatically sanitize #options by calling Xss::filter() on it (which removes dangerous markup) so not escaping the options would not be dangerous but role names are not really supposed to contain HTML so interpreting it here could be confusing for admins.
So I think this could be closed as wontfix.
@smustgrave I think #6 is about explaining a test failure, not steps to reproduce this issue.
The test failed because the previous patch removed escaping in Attachment.php and Feed.php which causes a breaking change.
(I did not keep changes to these files in the MR.)
We no longer ship web.config:
https://www.drupal.org/node/3440842 →
And according to #9 this can't be fixed in .htaccess.
So I guess we can't really do anything except document somewhere that we recommend disabling TraceEnable in Apache config?
Also the Apache doc (https://httpd.apache.org/docs/2.4/en/mod/core.html#traceenable) says this:
Despite claims to the contrary, enabling the TRACE method does not expose any security vulnerability in Apache httpd. The TRACE method is defined by the HTTP/1.1 specification and implementations are expected to support it.
So is this still a concern?
It would be nice to have a proof of concept.
Rebased and fixed failing tests.
If we can achieve to have just:
<input disabled>, then it can be a bit cleaner, but not sure if this can be achieved easily. Other option is to close this as Works as designed. Thanks!
Maybe something changed in core but I tested this and boolean attributes are displayed like you suggest.
prudloff → created an issue.
prudloff → created an issue.
prudloff → created an issue.
prudloff → made their first commit to this issue’s fork.
I had a look and only found this call that could easily be removed. But I might have missed some others.
I think several calls have been removed over the years ( #2569381: Drupal\views\Plugin\views\area\Result does an unnecessary XSS::adminFilter() → for example).
@duaelfr I'm not sure this is related. I think there was some issue about being able to set the list of allowed tags in view templates but I can't find it.
prudloff → created an issue.
The module has a Drupal 11 compatible release so marking as fixed.
The module has a Drupal 11 compatible release so marking as fixed.
The module has a Drupal 11 compatible release so marking as fixed.
The module has a Drupal 11 compatible release so marking as fixed.
The module has a Drupal 11 compatible release so marking as fixed.
prudloff → made their first commit to this issue’s fork.
prudloff → changed the visibility of the branch 2061377-image-style to hidden.
prudloff → changed the visibility of the branch 2061377-image-style-9.4 to hidden.
prudloff → changed the visibility of the branch 2061377-image-style-10.0 to hidden.
prudloff → changed the visibility of the branch 2061377-image-style-9.5 to hidden.
prudloff → changed the visibility of the branch 11.x to hidden.