Follow-up to #2273925: Ensure #markup is XSS escaped in Renderer::doRender()

Typically, non-HTML output would not go through the render API. An example could be json. If the json is going to include rendered output, how would that be supported?

Comments

xjm’s picture

Category: Bug report » Task
Priority: Normal » Major
Issue tags: +Needs issue summary update

Thanks @lokapujya for getting this filed!

xjm’s picture

Issue tags: -security, -SafeMarkup
joelpittet’s picture

IMO regarding JSON, it needs to remain as strings. So in the case of the SafeString objects that now exist, we need to ensure they are cast to (string) before encoded. For SafeMarkup, JSON should remain as-is and the consuming application will have to sanitize or not the values like in D7, they could be checked against the giant array of safe strings if within the same request, but I'd rather leave that assumption up to the consumer.

@lokapujya could you add other examples of non-HTML that come to mind?

Wim Leers’s picture

Title: Discuss how to support non-HTML output in the render API » Discuss how to support non-HTML output in the render system
Component: theme system » render system
lokapujya’s picture

render arrays, XML, FBML

I guess if another system or site trusts the Drupal output it wouldn't need to run it's own sanitization. It wasn't really my idea, I just filed the issue. "the consuming application will have to sanitize or not the values" - might be the way to go for now.

davidhernandez’s picture

Kind of agree with Joel's point of view here. And that is consist with the general Drupal philosophy that the data mostly stay untouched. But that is more of a philosophical/policy discussion than a technical one. I think the difference is that html output is something that we consider end-user facing output, and thus sanitize it. If we don't consider json, et al to be end-user facing, and I don't think we do, then don't sanitize it.

Question, though - Do we think that leaving the burden of sanitizing/safeguarding/whatever the data up to the consuming application is a hassle? I'm sure there are use-cases where it would be helpful to at least have the option of telling Drupal to only give me safe data.

Wim Leers’s picture

It's not just philosophical. If you get filtered/escaped data for a node's body field, you're no longer able to edit the data that is actually stored. So there's solid technical reasons for it too.

joelpittet’s picture

With this discussion are there some next steps to take? Are we currently sanitizing json or other format data that we shouldn't be?

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.