Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function source text serialization in error messages #49

Open
bathos opened this issue Dec 29, 2021 · 5 comments
Open

Function source text serialization in error messages #49

bathos opened this issue Dec 29, 2021 · 5 comments

Comments

@bathos
Copy link

bathos commented Dec 29, 2021

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
Copy link
Member

ljharb commented Dec 29, 2021

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
Copy link
Author

bathos commented Dec 29, 2021

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
Copy link
Member

ljharb commented Dec 29, 2021

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
Copy link
Author

bathos commented Dec 30, 2021

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
Copy link
Member

ljharb commented Dec 30, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants