Sorry, let me see if I can be more clear about the specific nature of my concern.
I see functions as being a fusion of two different kinds of things. One thing: they are algorithms. The algorithm executed by a function is immutably tied to its identity without us having to do anything.
The second thing they are is objects. A function can store keys and values, and most APIs that are expecting an object with properties foo and bar would also accept a function-object with properties foo and bar.
Here's the specific case that concerns me:
let { assign, freeze, isFrozen } = Object;
let doubleValue = (wrapper) => {
// The fix is to check isFrozen when typeof wrapper is function too
if (typeof wrapper === 'object' && !isFrozen(wrapper)) {
throw new Error();
}
let value1 = wrapper.value;
try { wrapper.value++ } catch (e) {}
let value2 = wrapper.value;
return value1 + value2;
};
let d1 = doubleValue(freeze({ value: 2 }));
let d2 = doubleValue(assign(function () {}, { value: 2 }));
console.log(d1, d2); // 4 5
This is what I mean: nothing about this demo in which I break the contract of frozen data involves a function which is peeking inside of the descriptors of objects.
Sorry, but that doesn’t resolve my confusion at all. Your proposed isDeepFrozen, Endo’s isPassable, and the existing Object.isFrozen all return true for primitive values, so filtering on typeof “object” is not just a bug but an unnecessary complication.
However, consider an object obj like { a(){}, get b(){} }—your deepFreeze implementation that ignores accessor functions introduces a difference between the result of an Object.isFrozen call with input Object.getOwnPropertyDescriptor(deepFreeze(obj), "a").value vs. Object.getOwnPropertyDescriptor(deepFreeze(obj), "b").get, despite both inputs being functions defined as part of the same “deeply-frozen” object. In fact, it isn’t even possible to write a predicate that will robustly differentiate a function that was originally defined as a method from a function that was originally defined as an accessor, making a deepFreeze that treats them differently even more surprising.
Well this is what's supposed to happen, we're supposed to talk until we're at least not confused by each other's ideas anymore.
I'm on the same page as you that filtering out primitives (before freezing) is likely an unnecessary complication. Essentially code that I had introduced as a perf optimization had created a bug, but I'm not even sure the optimization was meaningful. At very least it shouldn't introduce a bug!
I'm still not in agreement with what you're saying about property descriptors. You're checking to see if descriptor.get is frozen. To what end? What question is being answered? What pitfall prevented?
I'm also still unsure what reflection has to do with this. You can't cross into an object's descriptors by accident (as you can with prototype vs own properties, say). You have to do it on purpose with a call like Object.getOwnPropertyDescriptor.
I surely hope that I am open to persuasion, but I am not persuaded yet.
If it’s not frozen, it’s an information channel - someone can stick info on it and read it later. Immutability isn’t just about “the parts i care about won’t change”, it’s also “no part, including the ones i don’t know or care about, will change”.
OK, again that makes sense but leaves me wanting more. I'm reading up on HardenedJS and its usages. Seems like they're all smart contracts right now. BABLR would want to use it for zero-trust third-party plugins to an ecosystem of parsers, transpilers, linters, formatters and the like.
The closest thing HardenedJS mentions is the prevention of eavesdropping attacks as demonstrated in its security challenge: Web Challenge | Hardened JavaScript
These attacks make use of side channels, and so I now understand that HardenedJS goes to lengths to eliminate side channels.
The thing that still isn't fully resolved in my head is that I still think we're talking about a different kind of thing than a side channel attack here -- it'd be more like a back channel. If I were to set up a challenge to test for security against back channels I'd create two containers both with attacker code in them: one would have a MacGuffin, the other would have network access. The challenge now is to exfiltrate the MacGuffin with both containers working to break encapsulation. The defender wants to set the containers up in such a way that no information can move between them.
Is this really possible for the defender to win, and can someone confirm that this kind of security is a goal?
https://hardenedjs.org/ explicitly mentions “safe plugin systems”, along with “supply chain attack resistance” and “integrity in the face of adversarial code in the same process”.
A Taxonomy of Security Issues categorizes both “side channel” (unintentional, e.g. vulnerable timing measurement) and “covert channel” (intentional) as “non-overt”, and protection against both is in scope.
I'm at the point of really having to make a decision on this and as far as I can see I cannot get on board with harden as proposed.
Here are my problems with it:
First: It's named wrong. Freeze and harden are words that colloquially mean the same thing. I have a hard and fast rule for naming things that in a unified system of naming conventions if two closely related things have different names, it is imperative that the difference in the names communicates the difference in the things (and not just for people steeped in security jargon).
Second: This stuff with freezing the object properties of functions used as getters has tremendously complicated implications for the language as a whole. It's not well defined how much that might expand reachability.
Third: It isn't a standardized function!!! It has at least two completely different implementations, and all you know is that one of them is installed as Object[Symbol.for('harden')]. This forces the docs to note:
The first call to harden from any instance of @endo/harden determines the behavior of any subsequent instance of @endo/harden that initializes later, regardless of differences in behavior."
Or to quote from the comment in the source code:
/* This module provides the mechanism used by both the "unsafe" and "shallow"
* (default) implementations of "@endo/harden" for racing to install an
* implementation of harden at globalThis.harden and
* Object[Symbol.for('harden')].
*/
I just discovered this, and this alone makes it a non-starter for me. I'm not defining my environment to have two completely different functions race each other to take the same name!! That would cause insane bugs in my application that I could never fix. In addition no runtime could ever implement harden now as any single implementation that always loads first would supersede both other implementations, causing one half to get the other half's behavior: either safe becomes unsafe (very bad for SES), or unsafe becomes safe (and likely broken). Either way, it's a major breaking change to someone and the language can't evolve in a way that breaks the web. Since the name is poisoned and can no longer be used for anything standard, I guess I'm actually glad that it was the wrong name...
I actually misread the code comment: on closer reading it says the opposite of what I think it should say. It says: the "unsafe" and "shallow" implementations [race to install themselves on the global]
But "unsafe" and "shallow" mean the same thing in this context. It should warn that the "safe" and "unsafe" versions race to install themselves on the same name, though that's a bit of an odd way to state the real distinction. The real distinction is whether or not prototypes are frozen, down to and including Object.prototype, which comfortingly may or may not be locked down as a side-effect any time you call harden({}).
In defense of the authors of endo though, the one thing they actually don't create is Object.harden. So that name does remains free and clear for a standardized implementation which is good.
I think I lost the thread. What lazy computation issue are you concerned by? Does the existing harden do this lazy computation? (FWIW, I don’t think of harden as doing any lazy computation.)
In any case, we draw a distinction between “fundamental” integrity traits like “non-extensible” or “non-trapping” vs “emergent” integrity traits like “frozen” or “sealed”. The difference is that there is direct semantic state recoding whether an object is non-extensible. It only becomes non-extensible if prevent-extensions or equivalent. OTOH, an object is sealed if it happens to be non-extensible and all its own properties happen to be non-configurable. For example, if a non-extensible object has one configurable own property, then deleting that property would result in the object being sealed. This distinction is most directly evident in the proxy traps. There are proxy traps for isExtensible and preventExtensions but no traps for frozenness or sealedness.
A deep graph test is emergent to an extreme. Checking if a large graph of objects is deeply frozen or hardened might be very expensive. Fortunately, if the check succeeds, it can be memoized so repeating the check, or checking a graph that includes the memoized one, can avoid repeating that expense. But if the check fails, then the expense of repeated and related checks cannot be avoided.
deepFreeze or harden still has some of that expense problem. If they succeed, then they can (and harden does) memoize the success. If they fail, then there’s nothing to memoize and repeated attempts must pay the full costs. The difference is that failure is much more rare, failure is generally treated as exceptional, and therefore failed cases are much less likely to be repeatedly retried.
This is all completely orthogonal to the differences between deepFreeze and harden. It equally explains for both why I’m skeptical of adding a side-effect-free deep test.
@bergus a nit: By exporting the let binding, you’ve made it into a live binding, that is much harder to reason about. By exporting a const binding, an import cycle is still protected by temporal dead zone, but once initialized it is obviously guaranteed stable.
In the absence of compartments, we could have also chosen to race on an obscure or even symbol-named property on the global. But because of compartments, in our shim implementation, we instead chose an obscure symbol-named property on Object.
An open question for us is whether a proposal can somehow avoid proposing a race, and therefore a platform can avoid implementing a race. I hope so but I do not know.
@conartist6 , I want to note that we lived without the race for most of the history of harden. We introduced the race only recently, and we hate it! As far as we can tell, it was the least bad of a set of bad options forced on us. So if a proposed standard for something with direct platform support could avoid the race, we’d be overjoyed. The hard problem in this case was not eval twins. It was how to enable code to use harden so that code can practically run either in a non-locked-down environment and in a locked-down one, and to have harden actually provide useful protection in the latter case, where it is possible. In a non-locked-down environment, no meaningful protection is possible anyway, so we shouldn’t pretend otherwise. The salient distinction is how to handle inheriting from non-hardened objects.
We’re not stuck on the name. Any function that satisfies our needs + any name that is not misleading for that function might be fine with us. We’re certainly willing to bikeshed that. However, deepFreeze would be a misleading name for what we’re currently calling harden. The deepFreeze that you propose would not meet our needs, since it ignore inheritance.
Second: This stuff with freezing the object properties of functions used as getters …
Functions built to be used as function rather than constructor are rarely connected to surprising things. Generally only to Function.prototype. If they do accidentally have a .prototype property because they were defined with the function keyword but the code never mentions or uses this, then the value of that .prototype property is generally an empty of inheriting directly from Object.prototype. All this is especially true of getters and setters. We have not once encounter even any unpleasant surprising due to transitively hardening getters and setters.
Third: … It has at least two completely different implementations … non-starter …
I sympathize. See my previous comment above about the problem that we feel forced our choice. Note that a proposal can do things that a shim cannot because the proposal can assume platform support. I do not know if that helps here. But if any way can be found to escape this unpleasant choice that we can live with, then we’d be overjoyed.
Again, note that @bergus’s race, though only motivated by eval-twins, cannot help but allow a race winner that surprises someone. OTOH, if the only problem is eval-twins, then standardization alone would solve that. The harder problem is how to write code portable between locked-down and non-locked-down environments that are genuinely protected were possible: in locked-down environments.
which comfortingly may or may not be locked down as a side-effect any time you call harden({}).
If you’re in a non-locked-down environment, then prototype chains are not followed, so no implicit hardening of Object.prototype happens. If you are in a locked-down environment, then Object.prototype is already hardened.
This really is my primary concern so let's focus on this a second. I sort of understand what you're saying but my concerns are still quite large on this specific thing.
Even with your counterexample I can easily construct an example where the behaviors diverge:
class Foo {}
class Bar extends Foo {}
harden(new Bar());
// The result of this line depends on the race to install harden
Object.isFrozen(Foo.prototype);
You understand what I'm saying right? If there's two completely different behaviors with one name and it's the luck of the draw which behavior the name has in your environment, you'd basically have to feature-detect the specific implementation with a try/catch so that you know how to use it...
When using harden you should really be calling it on every "layer" of your prototype chain, and not rely on the implicit hardening of prototypes. In you example that's hardening Foo before you declare Bar. But it also means hardening instance when created, which is difficult to do with classes if you intent to allow them to be used as a base class.
The problem with the recursive behavior is a question of least surprise.
In a non hardened environment you cannot traverse prototypes because it's basically impossible to define which prototypes are primordial and when hardened would affect more than was intended.
In a hardened environment you want to be sure that after calling harden on an object, none of its surface is mutable. There are 2 ways to guarantee that: implicitly harden prototypes or throw if any prototype isn't pre-hardened. The latter would make it more likely for code that works in a pre-harden environment break when in an hardened environment. The former would make it possible as you describe that some code tested in hardened environment would not sufficiently harden when running in a non hardened environment.
My proposal to mitigate this is to console warn when in a hardened environment a call to harden would implicitly update a prototype that wasn't pre-hardened.
We could also have a heuristics for non hardened environment that warn when hardening an object which has a non-frozen prototype (avoiding the recursive walk of prototype objects) and that prototype is not one of the common JavaScript intrinsic prototypes.
Amazingly I hit an even worse snag with harden if that's even possible, and that's that the only thing you have to do to defeat either version of harden is use a getter.
Yeah, seriously, for all that hardening what you're left with still isn't immutable data, because any property could be a getter and hardened doesn't care. EVERY SINGLE INDIVIDUAL ACCESS of any hardened data MUST validate that it is not triggering a getter.
So yeah I can't use it and I can't recommend that anyone use it for anything at this juncture
I'm pretty sure I could use this to pwn almost any real SES application too, right?
In the SES security demo the API that's put out there as totally secure it never needs to pass any objects over a secure boundary, which is how the security demo page isn't vulnerable to an exploit even though the real tool would be