fix(ses): plug implicit NaN side-channel#3153
Conversation
🦋 Changeset detectedLatest commit: 10029d2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
erights
force-pushed
the
markm-no-implicit-NaN-leak
branch
2 times, most recently
from
692416d to
26cad3f
Compare
|
@ChALkeR , I cannot officially make you a reviewer, but I'd appreciate it if you could take a look. Thanks! |
erights
force-pushed
the
markm-no-implicit-NaN-leak
branch
2 times, most recently
from
2f0ddc4 to
4168d08
Compare
erights
force-pushed
the
markm-no-implicit-NaN-leak
branch
from
4168d08 to
7ed438d
Compare
kriskowal
left a comment
kriskowal
left a comment
There was a problem hiding this comment.
Approved with the caveat that we must publish this as a major version since it will break existing applications relying on the implicit availability of Float64Array in Compartments.
| }; | ||
|
|
||
| /** | ||
| * Replaces the dangerous `setFloat*` methods on `DataView.prototype` |
There was a problem hiding this comment.
@erights should we also update the getFloat* methods to something like const value = apply(…); return is(value, NaN) ? NaN : value? Not all ArrayBuffers will have been populated by SES code, and if anything it is probably more common to receive contents from the outside world.
There was a problem hiding this comment.
Wouldn't help. Either way, get produces a an opaque NaN. Only the setting side pierces the opacity.
3 participants
Closes: #XXXX
Refs: #XXXX
Description
The JavaScript language can leak the bit encoding of a NaN via shared TypedArray views of an common ArrayBuffer. Although the JavaScript language has only one NaN value, the underlying IEEE 754 double-precision floating-point representation has many different bit patterns that represent NaN. This can be exploited as a side-channel to leak information. This actually happens on some platforms such as v8.
@ChALkeR explains at tc39/ecma262#758 (comment) the behavior of this side-channel on v8. At https://junk.rray.org/poc/nani.html he demonstrates it, and it indeed even worse than I expected.
To plug this side-channel, we make two coordinated changes.
Float*Arrayconstructors as universal globals. This prevents them from being implicitly endowed to created compartments, because they are not harmless. However, we still keep them on the start compartment (the original global), consider them intrinsics, and still repair and harden them onlockdown(). Thus, they can be explicitly endowed to child compartments at the price of enabling code in that compartment to read the side-channel.lockdown(), we repair theDataView.prototype.setFloat*methods so that they only write canonical NaNs into the underlying ArrayBuffer.The
@endo.marshalpackage'sencodePassableencodings need to obtain the bit representation of floating point values. It had usedFloat64Arrayfor that. However, sometimes the@endo/marshalpackage is evaluated in a created compartment that would now lack that constructor. (This reevaluation typically occurs when bundling bundles in that package.) So instead,encodePassablenow uses theDataViewmethods which are now safe.Security Considerations
The point. This NaN side-channel in ses had been implicit, easily forgotten, and hard to plug in user code.
Scaling Considerations
Not really.
Documentation Considerations
Somewhere we need to document that the
Float*Arrayconstructors are not available in created compartments by default, and why.Testing Considerations
New tests included.
One test newly skipped because it always hangs for me locally, which is annoying. Reviewers, feel free to ask me to unskip it.
Compatibility Considerations
The absence of the
Float*Arrayconstructors in created compartments is potentially a compat break. In fact our need to repairencodePassabledemonstrates such a compat break.Upgrade Considerations
none.