Comments

DamienMcKenna created an issue. See original summary.

jlandfried’s picture

Status: Active » Needs review
StatusFileSize
new2.95 KB

Hello! I wanted to take a stab at this since I don't have a ton of experience writing tests, so any and all feedback is welcome.

In order to verify that the tests do indeed fail if xss was is not being filtered I temporarily set \Drupal\Component\Utility\Html::escape() and Drupal\Component\Render\PlainTextOutput::renderFromHtml() to both have a method body of return (string) $text;

Thanks! Hopefully this at least a decent start and is not too far off base.

jlandfried’s picture

StatusFileSize
new3.8 KB
damienmckenna’s picture

Parent issue: » #2788195: Plan for Metatag 8.x-1.0-beta11 release
StatusFileSize
new4.98 KB
new4.46 KB

Thanks for putting this together. I tweaked the image meta tag handling a little bit, and the output slightly.

  • DamienMcKenna committed c5cb575 on 8.x-1.x authored by jiff
    Issue #2796701 by jiff, DamienMcKenna: Added tests to cover XSS via meta...
damienmckenna’s picture

Status: Needs review » Fixed

Committed.

damienmckenna’s picture

Version: 8.x-1.x-dev » 7.x-1.x-dev
Status: Fixed » Patch (to be ported)

Needs to be ported to the D7 branch.

damienmckenna’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev
Status: Patch (to be ported) » Needs review
StatusFileSize
new3.72 KB

Some tests for the page title.

damienmckenna’s picture

Of course it'd help if I set it to test against the correct branch X-)

Status: Needs review » Needs work

The last submitted patch, 8: metatag-n2796701-8.patch, failed testing.

  • DamienMcKenna committed 837adfc on 8.x-1.x
    Issue #2796701 by DamienMcKenna: Added tests to cover XSS on the page...
damienmckenna’s picture

Status: Needs work » Fixed

The tests work against the 8.x-1.x branch :) Committed.

damienmckenna’s picture

Version: 8.x-1.x-dev » 7.x-1.x-dev
Status: Fixed » Patch (to be ported)

Back to needing to be ported.

damienmckenna’s picture

Issue tags: +Security
mariodan’s picture

StatusFileSize
new6.01 KB

Here is an attempt at making this xss test ported to the D7 branch. To verify the tests would fail if input was not sanitized, I disabled check_plain temporarily.

Please let me know if I should make any changes.
Thank you!

mariodan’s picture

Status: Patch (to be ported) » Needs review

Status: Needs review » Needs work

The last submitted patch, 15: metatag-xss_test-2796701-15.patch, failed testing.

mariodan’s picture

StatusFileSize
new6.01 KB

Here is that xss test ported to the D7 branch again. It think it was failing before because I created the patch wrong and a new file was not being created when the patch was applied in the test.

mariodan’s picture

Status: Needs work » Needs review
damienmckenna’s picture

damienmckenna’s picture

Duh, it was already committed to beta11.

damienmckenna’s picture

StatusFileSize
new6.14 KB
new3.12 KB

Minor tweaks.

damienmckenna’s picture

Status: Needs review » Fixed
Related issues: +#2758749: Plan for Metatag 7.x-1.18 release

Committed. Thanks for the backport, @DrupalDano!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.