Inline templates do not sanitize the inline variables properly.

This has slipped through the cracks since the test is not actually using an unsafe string. If I change the test to include some unsafe characters it fails:

  public function testInlineTemplate() {
    // ...
    $element = array();
    $element['test'] = array(
      '#type' => 'inline_template',
      '#template' => 'test-with-context {{ lama }}',
      '#context' => array('lama' => '<script>alert(\'oops\');</script>'),
    );
    $this->assertEqual(drupal_render($element), 'test-with-context ' . String::checkPlain('<script>alert(\'oops\');</script>'));
    // ...
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
Issue tags: +Quickfix
FileSize
2.91 KB

oh damn.

pfrenssen’s picture

dawehner’s picture

FileSize
3.01 KB
688 bytes

Let's harden it

pfrenssen’s picture

Patch looks good so far. Do you think it's a good idea to also update testInlineTemplate() to use an unsafe string?

pfrenssen’s picture

FileSize
3.01 KB

I deleted the patch from #3 accidentally. Here it is again.

dawehner’s picture

+1 for the idea in #4

pfrenssen’s picture

Assigned: Unassigned » pfrenssen
Status: Needs review » Needs work

I'll add it.

pfrenssen’s picture

Assigned: pfrenssen » Unassigned
Status: Needs work » Needs review
FileSize
4.19 KB
2.01 KB
chx’s picture

Title: Inline templates pass through unsafe strings » [sechole] Inline templates pass through unsafe strings
Status: Needs review » Reviewed & tested by the community

This looks great! I can't believe I missed this :(

dawehner’s picture

I have to say multiple times sorry, multiple times!

chx’s picture

This is the result of a manual merge went wrong in https://www.drupal.org/files/issues/2251113-70.patch -- but we caught it quite quick thanks @pfrenssen.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Good catch, better now than later.

Committed/pushed to 8.x, thanks!

  • catch committed 72450c6 on 8.0.x
    Issue #2330503 by pfrenssen, dawehner: Fixed [sechole] Inline templates...

Status: Fixed » Closed (fixed)

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

xjm’s picture

Issue tags: +SafeMarkup