Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
XSS in block titles, both in administration pages and in public pages.
Comment | File | Size | Author |
---|---|---|---|
#13 | block-1892504-13.patch | 4.17 KB | tim.plunkett |
#13 | interdiff.txt | 2.7 KB | tim.plunkett |
#9 | drupal-block_title_xss-1892504-9.fail_.patch | 2.92 KB | grisendo |
#9 | drupal-block_title_xss-1892504-9.pass_.patch | 4.36 KB | grisendo |
#2 | drupal-block_title_xss-1892504-6955968.patch | 1.43 KB | grisendo |
Comments
Comment #1
grisendo CreditAttribution: grisendo commentedI attach a patch
Comment #2
grisendo CreditAttribution: grisendo commentedComment #4
webchickThat is un-good. :(
Comment #5
nod_#2: drupal-block_title_xss-1892504-6955968.patch queued for re-testing.
Comment #6
grisendo CreditAttribution: grisendo commentedLooks like it passed now.
What's going on with autocomplete in d8? Can't make it work since long time ago...
Comment #7
grisendo CreditAttribution: grisendo commentedIt's OK to change issue status now?
Comment #8
chx CreditAttribution: chx commentedComment #9
grisendo CreditAttribution: grisendo commentedTests attached as patches.
Two patches: fail patch (tests only) and pass patch (tests + patch).
Comment #10
ParisLiakos CreditAttribution: ParisLiakos commentedpatch overall seems good to me, just
i am not sure if we do this anymore
Comment #11
grisendo CreditAttribution: grisendo commentedWhat do you mean exactly? I can see it at:
core/modules/block/lib/Drupal/block/Tests/BlockTest.php, line 22
and
core/modules/block/lib/Drupal/block/Tests/BlockTest.php, line 48
Comment #12
ParisLiakos CreditAttribution: ParisLiakos commentedi mean there is no reason for them to be properties.
Edit: i think i found an example in AggregatorTestBase
If i read this correctly all you have to do in construct is to login your user:
Comment #13
tim.plunkettHere it is, brought up to coding standards.
Comment #14
xjmWe probably want to add an additional template test similar to #1811684: XSS: Bartik's node.tpl.php prone to XSS (prints $title). I'll look at this.
Comment #15
tim.plunkettI think all of the entities could use some generic XSS checks similar to that other issue, but that'd just be additional coverage. This issue fixes the problem and has test coverage for the bug.
Comment #16
xjmOK, I'll make a followup.
Comment #17
xjmOh, for the record, #15 is not just @tim.plunkett RTBCing his own patch reroll. I reviewed it too on the 23rd. ;)
Comment #18
catchLooks good to me. I think it's OK to have a specific test for this. But yeah it wouldn't hurt to have similar "stick some XSS in here and see if it comes out somewhere else" tests for other entities at all.