Problem/Motivation

#2307505-31: Port twig_debug output to Drupal 7 brought up that the twig_debug output is not sanitized.

Proposed resolution

Add calls to \Drupal\Component\Utility\String::checkPlain() or similar.

Remaining tasks

Sign-off and commit

User interface changes

n/a

API changes

n/a

CommentFileSizeAuthor
#5 twig-debug-filter-2369781.pass_.patch5.16 KBlarowlan
#5 twig-debug-filter-2369781.fail_.patch2.11 KBlarowlan

Comments

xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Normal » Critical
Issue tags: +Security

Critical because unsanitized output is bad.

Is there a way we could have the output autoescaped by Twig, rather than adding checkPlain() calls?

cilefen’s picture

cilefen commented
Related issues: +#2340785: Create proper test method for determining if text has been escaped properly
dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +Needs tests

Ensure that we don't accidentally forget about it.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: Unassigned » larowlan
Issue tags: +CriticalADay

patch coming

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: larowlan » Unassigned
StatusFileSize
new twig-debug-filter-2369781.fail_.patch2.11 KB
new twig-debug-filter-2369781.pass_.patch5.16 KB
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Active » Needs review
Issue tags: -Needs tests

The last submitted patch, 5: twig-debug-filter-2369781.fail_.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Thank you @larowlan! I don't see anything missing here, test coverage looks good and all the variable bits of output are now escaped.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Good catch!

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 602a144 on 8.0.x 
    Issue #2369781 by larowlan: Ensure twig_debug output has needed...

Status: Fixed » Closed (fixed)

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