Making WeakMap security not depending on HIDDEN_NAME being neither unguessable nor undiscoverable

[kpreid]

Original issue 1444 created by bruant.d on 2012-02-04T11:55:37.000Z:

In this patch:

• I define LeakyWeakMaps (implemented with a key/value arrays pair)
• I replace the 2 key/values array pairs (hidden record and leaky weakmap for non-extensible objects) by LeakyWeakMaps

A natural consequence for hidden records is that the API they expose is the one of a weak map. If an attacker comes to know HIDDEN_NAME, the authority she's granted (with an object 'o' having HIDDEN_NAME as property) is equivalent to having access to a weakmap which keys are weakmaps using o as key. Consequently, unless an attacker knows already some weakmaps that use o as key, she cannot do anything useful. In case she does know some of thee weakmaps, she cannot do anything more than what she can do with the WeakMap.protoype API, so no additional authority is granted.

So, knowing HIDDEN_NAME does not provide more authority than not knowing it.

[kpreid]

Comment #1 originally posted by jasvir on 2012-09-08T01:21:22.000Z:

Thank you for the patch. @felix, when you get a chance can you test how significant the startup cost is from this patch. If it isn't, we should adopt the patch.

[kpreid]

Comment #2 originally posted by kpreid.switchb.org on 2013-06-18T21:31:03.000Z:

I've taken a look at this patch, and overall I think it is a great idea. However, as already noted, it has a flaw:

• * A case that has not been taken into account (yet) is when an
• * attacker creates such a property in a non-visited object yet.

In particular, consider this attacker, which uses only operations that cannot be monkeypatched:

var o = {}; o[HIDDEN_NAME] = { set: function evil(k, v) { ... } }; // now pass o to something which uses it in a WeakMap

The attacker now has:

1. stolen the WeakMap's internal leaky map (as k) (which is fixable easily enough, by using a different token)
2. stolen the value intended to be set in the map (as v), and
3. is able to cause the WeakMap to misbehave.

The first fix that comes to mind is to have some form of trademarking of the value of HIDDEN_NAME (that it is an authentic LeakyWeakMap). This would not work in the cross-frame case (e.g. a frozen object could only be put in WeakMaps belonging to the frame that froze it), but I think we can afford that in current Caja (there is one SES frame and it touches objects from its own frame and the host frame).

Any other ideas?

[kpreid]

Comment #3 originally posted by erights@google.com on 2013-06-19T00:15:04.000Z:

The other potential flaw it has is that it may be prohibitively expensive, given how intensely SES uses its own WeakMaps. I don't know that it is, but I just want to remind us that we need to test this before deciding to make this change.

[kpreid]

Comment #4 originally posted by erights on 2013-07-11T18:50:04.000Z:

<empty>

[kpreid]

Comment #5 originally posted by kpreid.switchb.org on 2013-09-10T17:17:28.000Z:

David Bruant, do you have any ideas about how to implement this without the problem I describe in comment # 2?

[kpreid]

Comment #6 originally posted by bruant.d on 2013-09-10T17:35:13.000Z:

I didn't find a better idea than trademarking LeakyWeakMaps (and throw if the value of HIDDEN_PROP isn't an authentic LeakyWeakMap). Not sure how much this costs though... I imagine you already know cheap ways to do trademarking? (the only thing I can think of in ES3 or 5 is storing all instances in an array and search it...)

@erights: I'm not too worried about performance. This patch is doing exactly what the previous one was doing. I just abstracted away the "keys" and "vals" arrays. Granted, in JS all abstractions cost, but usually marginally, not prohibitively. But then, there is the watermarking which I don't know how to do cheaply... In any case, agreed, it's probably something to test for.

[kpreid]

Comment #7 originally posted by bruant.d on 2013-09-10T17:39:01.000Z:

"This would not work in the cross-frame case (e.g. a frozen object could only be put in WeakMaps belonging to the frame that froze it)" I forgot to answer to this part. That's actually an interesting question to which I don't have the answer to. Is Caja capable of running script in an iframe one of its hosts has opened before the iframe own code? If so, then, you can alter the iframes Object.freeze. I currently don't even know if the platform can allow the parent of a frame to run something on the frame before the frame code executes.

[kpreid]

Comment #8 originally posted by kpreid.switchb.org on 2013-09-10T17:45:09.000Z:

Trademarking is itself built on using the trademarked object as a key in a WeakMap, so it is not an available tool unless we build a separate form of trademarking. I can't offhand think of a way to do that that doesn't either depend on an unguessable name itself or give the target object a chance to perform a side-effect.

[kpreid]

Comment #9 originally posted by bruant.d on 2013-09-10T17:57:45.000Z:

"I can't offhand think of a way to do that that doesn't either depend on an unguessable name itself or give the target object a chance to perform a side-effect." There is the expensive way:

var leakyweakmaps = []

function LeakyWeakMaps(){ leakyweakmaps.push(this); // patch code }

// ...later... var hiddenRecord = key[HIDDEN_NAME]; if(leakyweakmaps.indexOf(hiddenRecord) === -1) throw new Error('HIDDEN_NAME discovered and misused! Red Alert!!') // rest of the code

This is expensive because of the linear search... hmm... and I'm realizing leakyweakmaps are kept forever ruining the whole point of WeakMaps :-s

I'm out of ideas too for now. Sorry :-s

[kpreid]

Comment #10 originally posted by kpreid@google.com on 2013-11-07T20:30:08.000Z:

<empty>