LastPass: global properties can be modified across isolated worlds, allowing remote code execution

2017-04-02T00:00:00
ID SSV:92868
Type seebug
Reporter Root
Modified 2017-04-02T00:00:00

Description

A major part of the LastPass password manager is content scripts, additional privileged javascript that is injected into pages and can change or monitor content. LastPass use content scripts to search webpages for forms, add additional UI elements, and so on. The reason that it's safe to have content scripts with higher privilege than the page they're injected into is a concept called "isolated worlds". An isolated world is a javascript execution environment that shares the same DOM , but not variables and functions and so on.

Without isolated worlds, unprivileged pages could interfere with higher privileged scripts, and make them do whatever they want.

https://developer.chrome.com/extensions/content_scripts#execution-environment

It's important to remember that isolated worlds don't mean it's impossible to write insecure content scripts, it just means that it is possible to write secure content scripts; without isolated worlds it would be impossible.

https://developer.chrome.com/extensions/content_scripts#security-considerations

Consider an example content script like this: ``` var trusted=false

document.body.addEventListener("click", function() { if (trusted) { eval(window.location.hash.substr(1)) } }); ```

This is safe to inject unto untrusted pages, because pages cannot interfere with the value of the variable trusted, so cannot make the privileged script eval() something malicious.

Do we really need to declare the variable trusted? In JavaScript, it will simply be undefined, so couldn't we just do this?

document.body.addEventListener("click", function() { if (typeof trusted != "undefined") { eval(window.location.hash.substr(1)) } });

The answer is that we really do need to declare it. This is because although the page cannot define variables, the isolated worlds do share the same DOM, and DOM element ids automatically become properties of window.

https://html.spec.whatwg.org/#named-access-on-the-window-object

This means that the example script above is insecure, an exploit would simply look like this:

el = document.createElement("exploit") el.setAttribute("id", "trusted"); document.body.appendChild(el);

Now window.trusted is not undefined, and we can take over the privileged content script.

It looks like LastPass did not know that this was the case, because it's a common pattern in their extension to have a set of global flags that are undefined unless used. Here are some examples:

``` function lp_init_tlds() { "undefined" != typeof lp_all_tlds && null != lp_all_tlds || (lp_all_tlds = new Array) }

if ("undefined" != typeof g_sitepwlen && void 0 !== g_sitepwlen[e]) return g_sitepwlen[e];

"undefined" != typeof lpformfills && 0 == lpformfills.length && !m) { if (A) { var w = A.contentDocument; w && (w.m_abortedFormFillChecking = !0) } return !1 } ```

etc.

This is a very common pattern in LastPass, with hundreds of examples. This will require a considerable cleanup effort to find all cases where global properties are unsafely trusted.

Because untrusted websites can influence a bunch of codepaths in LastPass, it is very easy to turn this into remote code execution. I've created one example, but there are hundreds of places where attackers can influence codepaths in the LastPass extension that need to be cleaned up.

NOTE: Please do not release a patch until you're confident all cases have been fixed. Releasing a patch that just fixes the single case that I've made a demo for will make it very easy to identify the vulnerability and for someone to simply exploit any of the hundreds of others of cases where you've made this mistake. Please communicate with me on your plan to release fixes so that we can make sure the process goes smoothly. We will not release vulnerability details until either a patch is available or 90 days expire.


LastPass allows users to customize some settings from their online vault on lastpass.com. Their website communicates with the extension through a <form> with the special name "lpwebsiteeventform".

Here is the code that handles that, from onloadwff.js:

function handle_form_submit(e, t) { ... try { if ("lpwebsiteeventform" == t.name) lpwebsiteevent(e, t); else if ("lpmanualform" == t.name) lpmanuallogin(e, t); else { ...

This is secure, because they verify that the form is on one of the trusted domains "lastpass.com" or "lastpass.eu"

function lpwebsiteevent(e, t) { if (!lp_url_is_lastpass(document.location.href)) return !1; ...

How does lp_url_is_lastpass() work?

function lp_url_is_lastpass(e) { if (null == e) return !1; var t = /^https:\/\/([a-z0-9-]+\.)?lastpass\.(eu|com)\//i , n = "https://lastpass.com/"; if ("undefined" != typeof base_url && (n = base_url), &lt;-- XXX 0 == e.indexOf(n) || 0 == e.indexOf("https://lastpass.com/") || 0 == e.indexOf("https://lastpass.eu/")) return !0; if ("undefined" != typeof g_loosebasematching) { &lt;-- XXX var i = lp_gettld_url(e); return new RegExp(i + "/$").test(base_url) } return t.test(e) }

That seems reasonable, but look at the lines marked "XXX", global flags that might be undefined!

g_loosebasematching is a boolean flag, so just making it defined is enough. The untrusted page can do that like this

el = document.createElement("exploit") el.setAttribute("id", "g_loosebasematching"); document.body.appendChild(el);

But this still requires us to define base_url, and that is a string not just a boolean flag. An attacker can insert DOM elements and make base_url defined, but you can't make base_url.toString() contain anything useful, it will always have the value "[object SomeHtmlElement]". Even if we create ES customElements or make a class that extends HTMLElement, that wouldn't be reflected on other isolated worlds.

&gt; el = document.createElement("exploit") &lt;exploit&gt;​&lt;/exploit&gt;​ &gt; el.setAttribute("id", "base_url") &gt; document.body.appendChild(el) &lt;exploit id=​"base_url"&gt;​&lt;/exploit&gt;​ &gt; base_url.toString() "[object HTMLUnknownElement]"

I don't believe there is anyway to override HTMLElement.toTagString() that will work across isolated worlds, and all elements (HTMLTableElement, HTMLImageElement, HTMLUnknownElement, etc) work this way.

Well....there is one exception! HTMLAnchorElement.toString() works differently!