Questioning WeakMap.prototype.clear

Tom Van Cutsem tomvc.be at gmail.com
Tue Jan 22 23:27:45 PST 2013


I've been following the threads on WeakMap.prototype.clear with interest,
and I think I may have an argument that should make the existence of
WeakMap.prototype.clear irrelevant from an ocap security perspective.

It was already previously suggested in this thread that the ocap thing to
do would be to prevent access from untrusted code to your WeakMap instance
in the first place.

I think the suggestion that WeakMaps could be shared freely safely if only
they didn't have a .clear() method is wrong.

Consider for a moment that WeakMaps don't have a .clear() method so you
know no one can get at your values without having access to the
corresponding keys. So you share your WeakMap freely with untrusted code.
This still leaves you open to another attack. Before I can describe the
attack, I think it's useful if people have some understanding of what the
"rights amplification" pattern actually is.

The "rights amplification" pattern that MarkM previously referred to is
often used to convert an opaque, untrusted object reference into a value
your code can trust.

In my trusted code, I can write:

token = Object.freeze({});
myWeakMap.set(token, myTrustedObject);
return token;

I now pass around the token to code in the wild.

The token by itself is useless. It's only useful if someone later passes it
back to my code:

function doSomethingWith(token) {
  // token could be anything, I don't trust it
  var trustedObj = myWeakMap.get(token);
  // if trustedObj is not undefined, the token was legitimate
}


This pattern really hinges on the WeakMap being properly encapsulated. If
it isn't, an attacker could just mint his own tokens, add them to the
WeakMap, and then fool my trusted code into thinking that the token is
legitimate.

In other words: if one wants to share a WeakMap with untrusted code, one
must already wrap it such that access to the "set" method is prohibited as
well. In this regard, also having to remove .clear() is a marginal extra
cost, compared to the more general utility of this method when using
WeakMaps for caches rather than for rights amplification.

Cheers,
Tom

2013/1/22 David Bruant <bruant.d at gmail.com>

> Le 22/01/2013 15:59, Jason Orendorff a écrit :
>
>> A) Are there more WeakMap applications that will want .clear() or
>> applications that will want .clear() not to exist? Offhand I would bet on
>> the former, by a landslide, but if you think otherwise, or if there's some
>> other reason to privilege .clear() not existing, let's talk about that.
>>
>> (...)
>>
>>
>> Having said all that, I bet we could hack around the worst-case GC
>> performance. It'll be a pain, but GC is like that sometimes. This decision
>> should hinge on what provides the best API for developers. I think we
>> mainly disagree on what developers want, which is a great thing to talk
>> about. Let's talk about that.
>>
> I agree use case dominant is a crucially important answer. To date, I
> honestly don't know which case of want-clear and don't-want-clear would be
> dominant. I agree Allen showed a compelling use case but I can't judge how
> important it is. In my experience, I've not had the need for a .clear yet.
> I know that in some occurences, my code relies on the weakmap not being
> emptied, but I've kept the weakmap well-encapsulated in these cases so this
> experience is not that relevant (because intrusive .clear can't happen).
>
> I've written a whole paragraph which is the most factual I can say on the
> topic, but doesn't help the debate much.
>
> When it comes to feelings, I prefer prudence by default. I'd like to say a
> few words about that. I understand that features shouldn't be seen only
> with ocaps eyes, but I'd like to take a moment to describe what I care
> about when it comes to ocaps and what we call "security".
> Node.js is an interesting ecosystem. There are a lot of modules, it's not
> unusual to use 10-20 modules in a project which can make 100+ modules when
> counting recursively. Because it costs a lot of time, it's not possible to
> rewrite everything, it's not possible to contribute the necessary test
> coverage to modules and it's not possible to do careful security reviews of
> all used modules (and updates!).
> However, it's possible to apply POLA (Principle Of Least Authority), that
> is give to each module the information and capabilities it needs to do its
> job and no more. If WeakMap.prototype.clear gets natively in the language,
> it means *all* modules can have an irrevocable right to flush any weakmap I
> hand them.
> It's the same sort of problem than if a "free" operator was brought to
> JavaScript (in advance, I agree that the .clear is more acceptable) as
> suggested once [1] (almost ironically by Node's lead?). Suddenly, a module
> could free objects you hand out to them. A module thinks it's freeing one
> of it's own objects but actually frees one of yours because of a bug? Too
> bad, you'll be throwing a TypeError very soon and who knows in which state
> it will leave your application.
> Dave Herman made an equivalent case about coroutines [2]. It provides
> abusive authority: you call a module-imported function and for whatever
> good or bad reason, it can suddenly stop the stack. It makes your code
> harder to reason about because when you wrote the code, you probably
> expected the function call to return (or throw).
>
> Back to weakmaps, the issue here is not technical, but... cultural I would
> say. I can decide to encapsulate my weakmap in a WeakMapWithoutClear, but
> doing so, I cut myself from modules which take .clear for granted. A module
> does rely on WeakMap clearbility? It will hand me the weakmap I'm supposed
> to use and I know in advance anything can happen, because I didn't create
> this object.
> If weakmaps don't have a clear, modules using language weakmaps won't take
> it for granted and you can be fearless about sharing your weakmap... very
> much like you can be  hand objects today without the fear of them being
> free'd by mistake or malice using a free operator.
>
> Since I'm on the topic of language-based abusive authority, I've come
> across a case where a logging library would throw at my face [3] anytime it
> would have to log about an object with a cycle in it [4]. Logging libraries
> are really not the one you'd expect to throw an error so we didn't wrap the
> call in a try-catch. So logging some objects would make the application
> crash.
> I have come to think that error-throwing with stack-unwinding is an
> abusive authority. Especially given that try-catch is an opt-in.
> I understand now that silently swallowing errors is not a good idea and
> the need to report errors in a different channel than return value, but I
> don't think stack-unwinding is a good default for that. Promises show an
> interesting model; to keep in mind for another language maybe.
>
> The point I tried to make here is that POLA allows to build applications
> using untrusted modules without fearing them and that's really an excellent
> property, because we use constantly huge amounts code we don't necessarily
> trust or have the time to review or test besides what we test during the
> development period.
>
> WeakMap.prototype.clear is at a different scale of "danger" and abusive
> authority than a hypothetical free operator or coroutine or abusive
> stack-unwinding, but it's an issue of the same family.
> Whether the smaller scale of danger makes it acceptable is a good
> question. I would answer "no" by default, but I can understand if others
> say "yes" for this one. To be balanced with advantages that native
> WeakMap.prototype.clear provide which are yet to be accurately determined.
>
> David
>
> [1] https://mail.mozilla.org/**pipermail/es-discuss/2012-**
> October/026007.html<https://mail.mozilla.org/pipermail/es-discuss/2012-October/026007.html>
> [2] http://calculist.org/blog/**2011/12/14/why-coroutines-**
> wont-work-on-the-web/<http://calculist.org/blog/2011/12/14/why-coroutines-wont-work-on-the-web/>
> [3] https://github.com/flatiron/**winston/issues/151<https://github.com/flatiron/winston/issues/151>
> [4] https://github.com/flatiron/**winston/issues/100<https://github.com/flatiron/winston/issues/100>
>
> ______________________________**_________________
> es-discuss mailing list
> es-discuss at mozilla.org
> https://mail.mozilla.org/**listinfo/es-discuss<https://mail.mozilla.org/listinfo/es-discuss>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20130123/9c9fdd9f/attachment-0001.html>


More information about the es-discuss mailing list