Proposal for 1.0 - Remove/deprecate os:Html tag

[goosemanjack]

Synopsis:

The os:Html tag adds complexity to the spec without providing any real
additional value in terms of security. Because of this, we propose
os:Html be deprecated from the 1.0 version of the spec.

(section proposed for deprecation)
http://www.opensocial.org/Technical-Resources/opensocial-spec-v09/OpenSocial-Templating.html#rfc.section.11

The intent of the os:Html tag is to provide for strict escaping rules
for rendering markup from untrusted or trusted sources. In practice
this forces a specific and sub-optimal security model for
implementers. Its protections are easily circumvented by using a
variety of other techniques, like gadgets.io.makeRequest, direct DOM
manipulation, or combinational attacks

ex:
Four requests each have data that is not problematic in isolation.
When placed together they construct an attack vector.

v1: <sc
v2: ript>alert('I just XSSed your bank account')<
v3: /s
v4: cript>


No single variable contains an obviously outlawed tag, but place them
together:

<os:Html code="${v1}" /><os:Html code="${v2}" /><os:Html code="${v3}" /
><os:Html code="${v4}" />

and they become:

<script>alert('I just XSSed your bank account')</script>


There are multiple more appropriate locations for providing security
filtering, either before or after the rendering:

* in the proxy layer being called by the request
* on the receiver of an <os:HttpRequest> tag when @format is specified
as "text"
* in the template renderer
* with a meta-sandbox, like Caja or Web Sandbox


Security is a difficult topic and the OpenSocial spec should not be
getting into the business of defining the security model a particular
implementer will use. That should be left to the implementer so that
we can allow for advances in security techniques and explore different
security implementations. Because of these reasons and the
shortcomings of the <os:Html> tag, we feel it should be removed from
the OpenSocial specification.

[Evan Gilbert]

The goal of <os:Html> is to create an easy syntax for rendering HTML sent as part of data.

${foo} will escape the contents of foo, so "<b>Hello</b>" would turn into &lt;b&gt;Hello&lt;/b&gt;

Think we still need <os:Html> to support output of Hello in this case.

[goosemanjack]

Auto-escaping (i.e. HTML encoding of all data) of all response data is
not something I find called out in the spec, except tangentially in
the os:Render tag, which IMO should also be deprecated. HTML encoding
appears to be an implementation decision made by the Shindig team and
is not one we're planning on following in the MySpace stack, given
current information on how developers are using the MySpace platform.
--
clc

On Oct 7, 9:34 am, Evan Gilbert <uid...@google.com> wrote:
> The goal of <os:Html> is to create an easy syntax for rendering HTML sent as
> part of data.
>
> ${foo} will escape the contents of foo, so "<b>Hello</b>" would turn into
> &lt;b&gt;Hello&lt;/b&gt;
>

> Think we still need <os:Html> to support output of *Hello* in this case.

>
> On Wed, Oct 7, 2009 at 2:39 AM, goosemanjack <cc...@myspace.com> wrote:
>
> > Synopsis:
>
> > The os:Html tag adds complexity to the spec without providing any real
> > additional value in terms of security.  Because of this, we propose
> > os:Html be deprecated from the 1.0 version of the spec.
>
> > (section proposed for deprecation)
>

> >http://www.opensocial.org/Technical-Resources/opensocial-spec-v09/Ope...

[Lev Epshteyn]

The last paragraph of the section on expression evaluation specifies how strings are escaped:

http://www.opensocial.org/Technical-Resources/opensocial-spec-v09/OpenSocial-Templating.html#rfc.section.6

Given this, I don't see how <os:Html> can be dropped (without changing this part of the spec). I think auto escaping of ${} is useful, because it protects template layouts from markup where none should be allowed. Otherwise, what's to keep people from inserting a large <img> into their displayName?

The attack you propose would indeed compromise an implementation that checked individual data. It wouldn't affect Shindig on the server, because we inspect and sanitize the document as a whole after templates have been rendered. On the client, each <os:Html> tag would be hosted in its own <span>, so again such an attack would fail.

[goosemanjack]

I see I am in error. There is text about how to escape strings. Had
we caught this, it is something we would have challenged in the 0.9
version of the spec since escaping rules will often create more
problems than they solve.

(http://www.opensocial.org/Technical-Resources/opensocial-spec-v09/
OpenSocial-Templating.html#rfc.section.6)
Strings are escaped before inserting into the HTML document. By
default strings are HTML escaped,
however based on context they may be URL encoded or escaped as
JavaScript strings or CSS content.

I'll work up a proposal to fix the escaping rules to something we can
comply with. The proposal will also contain language clarifying how
we think EL variables should evaluate.
--
clc

[goosemanjack]

Given that Shindig sanitizes server-side post-render (which I also
believe to be the correct approach), the value of auto-escaping seems
lessened. I state this only because I'll be building a case to
partially or completely remove the language about escaping EL variable
content.
--
clc

On Oct 8, 6:49 am, Lev Epshteyn <le...@google.com> wrote:
> The last paragraph of the section on expression evaluation specifies how
> strings are escaped:
>

> http://www.opensocial.org/Technical-Resources/opensocial-spec-v09/Ope...

[Lev Epshteyn]

The shindig sanitizer removes dangerous things - mostly related to javascript. It doesn't really know when markup is not appropriate - as the template/app developer undoubtedly does.

Consider a simple friend list usecase: <os:Repeat expression="${people}">${Cur.name}</os:Repeat>.

If I set my name to "<div style='width:3000px; height:3000px'>lev</div>", there is no way for shindig to know this markup is malicious in the context it's set. However, there is no doubt that it will break the layout of any page that tries to display my name.

While more appropriate tags exist for rendering a name (such as <os:Badge/>), this problem exists for any user-defined field.

Having both ${} escaping and <os:Html> gives the developer a choice of when to allow HTML markup, and when not to.

Moreover, it gives us an extension point for more granular control in the future - for example: <os:Html allow="inline"/> could be used to allow only inline style markup (such as bold, italic, colors, images that are smaller than 1em in height), but not block-level markup that would potentially break layout.

[Adam Winer]

-1 to removing os:Html for all the reasons stated above.

The server-side sanitizer is not relevant; it's a large hammer to
swing, and not a sufficient solution in general.

Auto-escaping of ${} is critical. Templating languages that don't
escape content by default are XSS attacks waiting to happen. This has
to stay in.

The problematic example given above - os:Html with "<sc", "ript>",
etc., is one which is trivially addressed - "<sc" should be treated as
a text block, not an element, and should be escaped as "&lt;sc" on
output. If Shindig doesn't do that already, it's a Shindig bug.

-- Adam

[goosemanjack]

> Having both ${} escaping and <os:Html> gives the developer a choice of when
> to allow HTML markup, and when not to.

Inverting the decision on when to escape and when not to escape seems
a more appropriate path - so that things are not escaped by default,
but functions can be called to perform escaping in contexts where the
developer knows they are using untrusted content. In my experience
auto-escaping is a source of more problems than solutions. Think
magic quotes in PHP.

I'm going to consult with our security team as to if auto-escaping
truly is providing correct protection against an XSS attack, esp given
that we also provide (and assumedly many, if not most developers will
be forced to use) the un-escaping tag os:Html. It comes down to an
80/20 rule. Most of the time our developers need to be able to inject
markup in a more-or-less unconstrained manner. Given the cross-server
functionality we have to support to allow app developers to keep
parity with what they can already do, differentiating between an XSS
attack and a legitimate mashup is non-trivial.

At this moment I'm still of the position that forcing the auto-
escaping/tag-based unescaping as a security solution 1) incorrectly
constrains how security is implemented in OSML apps and 2) is the
wrong solution. This position could change, of course. I'll get back
to this thread when I have the feedback from our security team
(probably next week).
--
clc

[Adam Winer]

On Fri, Oct 9, 2009 at 4:07 AM, goosemanjack <cc...@myspace.com> wrote:
>
>> Having both ${} escaping and <os:Html> gives the developer a choice of when
>> to allow HTML markup, and when not to.
>
> Inverting the decision on when to escape and when not to escape seems
> a more appropriate path - so that things are not escaped by default,
> but functions can be called to perform escaping in contexts where the
> developer knows they are using untrusted content.  In my experience
> auto-escaping is a source of more problems than solutions.  Think
> magic quotes in PHP.

My experience, both at Google and prior to that, is entirely the
opposite. Unless the easiest possible path prevents XSS attacks,
developers *will* introduce XSS issues routinely. An inconvenient and
explicit back-door for bypassing auto-escaping is the right approach.

> I'm going to consult with our security team as to if auto-escaping
> truly is providing correct protection against an XSS attack, esp given
> that we also provide (and assumedly many, if not most developers will
> be forced to use) the un-escaping tag os:Html.  It comes down to an
> 80/20 rule.  Most of the time our developers need to be able to inject
> markup in a more-or-less unconstrained manner.

No, most of the time they don't. Developers constantly need to insert
untrusted data (e.g., profile fields) that should not be allowed to
contain markup.

This isn't guesswork: prior to templating, developers used JS to
generate content. Most developers got escaping wrong, and that led to
a constant stream of real-world XSS attacks. Going back to insecure
by default would be a terrible choice.

[Lane LiaBraaten]

Sounds like there are separate use cases for ${} (escaped) and <os:Html> (unescaped), so let's call this proposal out of scope for this iteration.

-Lane

[Tim Moore]

On Oct 8, 6:49 am, Lev Epshteyn <le...@google.com> wrote:

> Otherwise, what's to keep people from inserting a large <img> into their
> displayName?

This example concerns me. It implies that containers aren't escaping
the values of gadgets' metadata attributes before displaying them. If
the substitution of a value into displayName is escaped, and the
container is then escaping the value of displayName, then you'll end
up double-escaping legitimate titles in some cases.

On the other hand, if the substitution is escaped and the value of
displayName is not escaped by the container, then there's nothing
stopping a gadget author from just putting the <img> into the spec
directly without using templates.

I just looked through the spec for clarification and I can't even find
a part where it specifies that you can use expressions outside
<Content> at all. I know this has been implemented in practice, so the
spec should really be updated to reflect how it works.

-- Tim

[Adam Winer]

On Sat, Oct 10, 2009 at 10:13 AM, Tim Moore <tmo...@atlassian.com> wrote:
>
> On Oct 8, 6:49 am, Lev Epshteyn <le...@google.com> wrote:
>> Otherwise, what's to keep people from inserting a large <img> into their
>> displayName?
>
> This example concerns me. It implies that containers aren't escaping
> the values of gadgets' metadata attributes before displaying them. If
> the substitution of a value into displayName is escaped, and the
> container is then escaping the value of displayName, then you'll end
> up double-escaping legitimate titles in some cases.

We're not talking about gadget metadata here. We're talking about
data from the "person" endpoint. Two separate things.

There's no built-in way to display the gadget author that I know of
(certainly no spec-defined way to get at it in the expression language
variables).

> On the other hand, if the substitution is escaped and the value of
> displayName is not escaped by the container, then there's nothing
> stopping a gadget author from just putting the <img> into the spec
> directly without using templates.

We're talking about templates here.

> I just looked through the spec for clarification and I can't even find
> a part where it specifies that you can use expressions outside
> <Content> at all. I know this has been implemented in practice, so the
> spec should really be updated to reflect how it works.

I'm not aware of it being implemented; Shindig, at least, only
supports expressions in template blocks and data pipelining blocks.

-- Adam

>
> -- Tim
> >
>

[Tim Moore]

Sorry, ignore me... I was totally confused. I haven't dived into the
templates stuff very seriously yet (our implementation is still 0.8-
based) and I completely misunderstood what you were saying (mistaking
displayName for the gadget title attribute).

OTOH, I was under the (mistaken?) impression that templates were meant
to replace hangman variables wherever they can occur. That's not the
case?

[Adam Winer]

On Mon, Oct 12, 2009 at 1:01 PM, Tim Moore <tmo...@atlassian.com> wrote:
>
> Sorry, ignore me... I was totally confused. I haven't dived into the
> templates stuff very seriously yet (our implementation is still 0.8-
> based) and I completely misunderstood what you were saying (mistaking
> displayName for the gadget title attribute).
>
> OTOH, I was under the (mistaken?) impression that templates were meant
> to replace hangman variables wherever they can occur. That's not the
> case?

Template EL is a better version of hangman variables, and I'd like a
future version (post 1.0) of OpenSocial to support EL where-ever
hangman variables might occur, and deprecate/de-support hangman
variables. If we added such support, I'd argue that it'd have the
same auto-escaping as it does in templates.

-- Adam Winer