tc39 / proposal-function-implementation-hiding

JavaScript language proposal: function implementation hiding
https://ci.tc39.es/preview/tc39/ecma262/pull/1739
MIT License
98 stars 7 forks source link

Function source text serialization in error messages #49

Open bathos opened 2 years ago

bathos commented 2 years ago

In V8, some error messages currently expose the source text of an ES-code function.

try { Uint8Array.from.call(x => x + 3); } catch (err) { err.message; }
→ 'x => x + 3 is not a constructor'

(Catching and reading the message property is to make it clear we’re looking at the real message string value, not something specific to the the enhanced outside-the-runtime debugging representation of error values used in the console.)

The current spec text’s new “forbidden extension” clause may already be general enough to address this:

Runtime mechanisms for inspecting Error objects must not expose the existence or source code location of function implementation hidden code. For example, a "stack" property which gives a stack trace must not include such functions in the stack trace. (This is not in any way meant to restrict debugging tools, or other non-runtime introspection mechanisms.)

However it seemed worthwhile to put it on the radar in case it wasn’t already known that some engines render error messages that form side channels for (sometimes otherwise-unknowable) information about arbitrary values. The channel here is neither stack nor Function.prototype.toString, which is not called.

Even in the absence of this specific proposal, I think there is already a pretty strong case that this should be considered a kind of misbehavior. It shows up in a few other places in V8 that would not be addressed by this proposal, too. For example we can obtain the original name of a constructor which is not available using any specified information channel:

class Hidden {}
delete Hidden.name;
Hidden = Hidden.bind();
delete Hidden.name;
Reflect.getOwnPropertyDescriptor(Hidden, "name");
→ undefined

try { Uint8Array.from.call(new Hidden); } catch (err) { err.message.slice(2, -22); }
→ 'Hidden'

(Again, to be clear, it is not weird or problematic for the console to show Hidden as the label — it’s that it is being exposed in a runtime context as an ES string where, with source text redaction or plain-old bind(), nothing could have otherwise obtained it.)

AFAIK, Firefox/Spidermonkey does not create any side channels through its error messages. Instead it only uses information that is potentially available within the ES runtime context when constructing its messages. It's hard to say for sure if this is 100% true given the open-ended nature of the messages; there could easily just be cases I just haven’t found since the range of things to try is pretty much infinite. In V8, many similar cases don’t render value representations like these and I don’t know if there’s a pattern that would explain why some do and some don’t.

ljharb commented 2 years ago

Including the source text doesn't tell you the source text location. If there's an issue in v8, a bug should be filed there, but this doesn't seem like a spec issue.

bathos commented 2 years ago

Including the source text doesn't tell you the source text location.

The forbidden extension text in this proposal says “expose the existence or source code location”. I figured “existence” included the source text itself, but I’m not sure if that’s correct given it’s odd phrasing. Are you saying that this sentence is supposed to only forbid exposure of location and not also the source text?

If so ... shouldn’t it? What use is "hide source" otherwise?

If there's an issue in v8, a bug should be filed there, but this doesn't seem like a spec issue.

I likely communicated this poorly, but nothing shown above is, to my knowledge, a bug. Even if this proposal succeeds, nothing new would be forbidden except where the directives apply — and then only if my interpretation of “existence” was sound, which now seems shaky. While I personally think it would be great if non-F.p.toString exposures were forbidden across the board regardless of what directives are used, it doesn’t seem like anything constrains this today.

That it’s currently compliant is what makes it notable vis a vis "hide source".

ljharb commented 2 years ago

It seems like the exposure of the name “Hidden” on an instance, when it’s no longer present on the constructor (bound or not) except in the source text (which is only used per spec to set the initial name) is a bug.

bathos commented 2 years ago

Interesting, I want that to be considered a bug, but I didn't think it was actually mandated by anything. I'll open a Chromium issue for that, but - do you know if there's something normative that forbids it? Perhaps they'll agree it's a mistake regardless of whether the requirement exists, but it would help to be able to point to something that says outright that implementations aren't permitted to expose reflection capabilities that reveal information about ECMAScript source code that would not otherwise be retrievable via a mechanism specified by ECMA-262 (or something like that with an equivalent effect).

ljharb commented 2 years ago

ohh, i see, it’s coming from the error message/stack. No, there’s nothing that forbids it, true - but it still seems like a bug.