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

Interaction with stacks? #8

Closed ljharb closed 5 years ago

ljharb commented 6 years ago

How does a stack frame from within a censored function show up in a stack trace?

How does a stack frame from within a non-censored function (but that is "within" a censored function in the call stack) show up in a stack trace?'

Not sure what the answer should be, but these questions definitely need answering prior to stage 3.

(https://github.com/tc39/proposal-error-stacks)

cc @erights @codehag

michaelficarra commented 6 years ago

I think it should be unaffected. According to my understanding, this should affect Function.prototype.toString results only. If it has a broader goal, we should rename the proposal and the pragma. "hide implementation"?

ljharb commented 6 years ago

That's one possibility, to be sure. However, I think needing to report a line number might conflict a bit with not holding onto any source text?

michaelficarra commented 6 years ago

I'd be surprised if source text was needed for that, but I also don't maintain an implementation that tracks position information at runtime.

caridy commented 5 years ago

@ljharb I think that should work the same way native code works today, e.g.:

a.forEach((v) => {
    external(v)
})

If external method throws, you still have such information in the error stack, even though it is invoked from within the forEach intrinsic, which is native, e.g.:

VM368:2 Uncaught Error: external
    at external (<anonymous>:2:11)
    at a.forEach (<anonymous>:4:4)
    at Array.forEach (<anonymous>)
    at <anonymous>:1:3
ljharb commented 5 years ago

@caridy that doesn't give any line numbers inside of forEach tho; just the name of the function that took the callback. iow, there's a frame "missing", between line 2 and 3.

caridy commented 5 years ago

@ljharb yes, that's the model that works better IMO. In other words, for me that's not a "missing" frame, but an intentionally "removed" frame.

ljharb commented 5 years ago

@caridy agreed - so the question remains, should those frames be intentionally removed from functions that are censored?

michaelficarra commented 5 years ago

At the last TC39 meeting, following the discussion of Error stack censorship, I spoke to @phoddie about the relationship between stack frame censorship and this proposal. After what we learned from the plenary discussion1 and my discussion with @phoddie, I'm convinced that not only should these efforts be combined under a single goal of preventing source text related information leakages, but that these should be enabled by a single directive. A JS developer who cares about one of these information leakages will likely care about the other, as well as any we discover and choose to include in the future. We can't expect them to make decisions about individual security properties they want to maintain or to be constantly keeping their codebases updated with new security-related directives.

@domenic What do you think about expanding the scope of this proposal to encompass all implementation hiding? For now, that would just include omitting the frames of annotated functions from stack traces. I think that "hide implementation" would be a pretty good name for this directive.

1 a directive for stack censorship would be better than an API (like Error.hideFromStacktraces(fn)) to avoid allowing maliciously censoring others' functions from stack traces

domenic commented 5 years ago

@michaelficarra that sounds great to me! I'd welcome your help updating the proposal, readme, etc. in that direction, although I hope to also make some updates myself in order to prepare for the next meeting.

michaelficarra commented 5 years ago

@domenic Sure, I'll send a PR.