HTML injection issue for Internet Explorer users

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Embed an swf providing a flashvars argument such as {test:
'"/></object>...more html...'}
2. Open the page with an Internet Explorer browser

What is the expected output? What do you see instead?

The swf fails to load and the html from the flashvars is injected into the
page.

What version of the product are you using? On what operating system?

Swfobject 2.2
Internet Explorer 7/8

Please provide any additional information below.

This is actually the same issue reported on
http://code.google.com/p/swfobject/issues/detail?id=402

As far as I know, only IE users are compromised. In Firefox, Safari and
Chrome, the param tag for the flashvars is properly escaped.

Original issue reported on code.google.com by malterisio777 on 14 Jan 2010 at 6:08

[GoogleCodeExporter]

Merging this into issue #66, as the root cause of the security hole is unescaped
flashvars.

Original comment by platelu...@gmail.com on 14 Jan 2010 at 11:44

• Changed state: Duplicate

[GoogleCodeExporter]

The attachment is a patch against the trunk for this issue.

Actually, all variables from the object holding the parameters are encoded with 
this
patch, mirroring what happens when the DOM is used to generate the HTML. 
Variables
from the object holding the attributes should also be escaped but it's not 
included
in the patch (although is quite trivial to also do that).

The escaping is done only for the " character, which is converted to ". This is
enough to stop the HTML injection in IE.

Original comment by malterisio777 on 15 Jan 2010 at 1:23

Attachments:

• swfobject-fix427.patch

[GoogleCodeExporter]

Upon further investigation, the scope of this issue isn't just limited to the
flashvars, all the parameters are unsafe:

swfobject.embedSWF('"></object>...more html...', "myContent", "300", "140", 
"9.0.0",
"expressInstall.swf");

swfobject.embedSWF("test.swf", "myContent", '"></object>...more html...', "140",
"9.0.0", "expressInstall.swf");

This all will result in the player failing to load and html being injected when 
using
an IE browser.

Original comment by malterisio777 on 15 Jan 2010 at 2:02

[GoogleCodeExporter]

I looked a bit at the patch, and it seems OK to me. Escaping values before 
putting
them in an attribute between quotes is really the way to go.

However, I would also suggest escaping at least &, and probably < and > as 
well. The
former prevents problems when one of the flashvars already contains HTML 
entities.
For example, if I pass a flashvar containing "Examples of & and other entity
declarations", I expect that this will end up at the flash player as "Examples 
of &
and other entity declarations" (or perhaps even break due to #66).

Escaping < and > doesn't seem critical, but it seems common to do it anyway 
(and it
shouldn't hurt).

As you suggested already, the other parameters should also be escaped, but I 
guess
the same function can just be reused.

Original comment by matthijskooijman@gmail.com on 21 Jan 2010 at 5:03

[GoogleCodeExporter]

Un-merging from issue 66 by popular demand.

Original comment by platelu...@gmail.com on 22 Jan 2010 at 2:59

• Changed state: Accepted

[GoogleCodeExporter]

This isn't a security issue. This requires that the user already has access to 
the page 
html or Javascript, and if they already have that, why would they bother 
injecting things 
through swfobject? Site owners need to sanitize user input on their own.

Lets keep the discussion as to whether to automatically escape the 
flashvars/params in 
issue 66.

Original comment by TenSafeF...@gmail.com on 19 Feb 2010 at 5:36

• Changed state: Invalid

[GoogleCodeExporter]

Thank you for all your input, malterisio777 and  matthijskooijman

We've discussed the issue, and the consensus of the team is that the HTML 
injection
vulnerability via SWFObject is no different than it would be with any major JS
framework, including the ever-popular jQuery. As Geoff mentioned, the hacker 
would
have to have write access to the page, and at that point they could bypass 
SWFObject
completely.

We agreed SWFObject should probably ensure that vars passed via querystrings are
sanitized. However, that is not the topic of this issue, so this issue will 
remain
closed.

Original comment by platelu...@gmail.com on 19 Feb 2010 at 7:07

[GoogleCodeExporter]

I still think I don't agree here. You're saying " This requires that the user 
already
has access to the page html or Javascript", but AFAICS it is different: If a 
user has
access to the content of the flashvars (which is not unreasonable, say when 
passing a
movie title to Flash), he can use swfobject to gain access to the page's HTML, 
insert
<script> tags and perform XSS attacks.

Perhaps your (implicitely) arguing that the encodeURIcomponent escaping to 
solve #66
will prevent this flaw from being exploited, but the code is still wrong and 
might
cause unexpected behaviour (at the very least, the HTML generated is incorrect,
because of the ampersands used when joining the flashvars together).

Original comment by matthijskooijman@gmail.com on 19 Feb 2010 at 9:08

[GoogleCodeExporter]

I disagree with the decision, it still seems that the nature of the issue 
hasn't been
fully grasped. It is not about uriencoding the individual flashvars, it is about
swfobject generating the html in an unsafe and incorrect way for Internet 
Explorer users.

Original comment by malterisio777 on 19 Feb 2010 at 12:34

[GoogleCodeExporter]

We've just encountered the very same problem in one of our projects. 
Actually it's not a security hole but a broken implementation of embedding.

In case you want to embed any text to a Flash movie that contains quotes, IE 
fails:

playervars = {'caption': 'and then someone told me "hey this just do not 
work!"'};
swfobject.embedSWF('xyz.swf', 'playercontainer', '950px', '530px', '9.0.115', 
  'flash/expressInstall.swf', playervars, params, playerLoaded 
);

The above setting leads to the string broken into attributes (see attached 
image of IE inspection). This is NOT the question of server side encoding, but 
a broken implementation - the very same parameters work flawlessly in non-IE 
browsers due to the different way of passing parameters.

Additionally, the patch posted above works only for string parameters, for 
non-strings (eg. a boolean) an exception is thrown.

Original comment by turcsa...@gmail.com on 8 Jun 2010 at 5:17

Attachments:

• Clipboard-1.png