endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
829 stars 72 forks source link

v8 error taming should leverage `prepareStackTrace` #1798

Open mhofman opened 1 year ago

mhofman commented 1 year ago

What is the Problem Being Solved?

Error.prepareStackTrace can be used by Node.js to apply source maps to the captured stack trace. However SES sets its own prepareStackTrace during v8 Error taming. Instead of completely discarding it. Maybe instead it should leverage as much as possible the existing prepareStackTrace to prepare its own stack strings?

Furthermore, if the start compartment set a prepareStackTrace after lockdown, it seems the returned value will be used for the .stack property instead of the empty string, even if errorTaming is set to 'safe': https://github.com/endojs/endo/blob/2a85f8aa6684b4d4a92fea48ccc63d01ccddc85e/packages/ses/src/error/tame-v8-error-constructor.js#L301-L304

Description of the Design

Use the original prepareStackTrace to generate the stack string.

Alternatively implement source map support in endo itself (maybe once part of the error handling logic has been ejected from SES as a trusted shim?, see #945). While node's prepareStackTrace implementation is not very modular, it should be possible to rebuild it on top of findSourceMap.

Security Considerations

None particularly. The start compartment is already in position to decide how lockdown should tame errors, and can also set prepareStackTrace after lockdown

Scaling Considerations

None

Test Plan

load some minified source which throws errors in Node.js with --use-source-maps and verify stack traces are mapped.

Upgrade Considerations

None particular

erights commented 4 months ago

Hi @mhofman , I've co-assigned myself to this.

mhofman commented 4 months ago

Use the original prepareStackTrace to generate the stack string.

I believe I explored this, and that didn't work