1827576 - RFP Math sin/cos/tan spoofing missing for asm.js

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P2)

Description

[Tom Schuster (MoCo)]

While looking over the patch in bug 1823880 again, I noticed that we seem to have missed asm.js code for directly calling the math sin/cos/tan native function even in the initial landing of bug 531915.

We should fix this and also add a test. (Actually also for WASM)

Comment 1

[Tom Schuster (MoCo)]

It doesn't look like the AsmJS code is setup in a manner that would make selecting a different implementation of the math functions based on the function's realm easy to implement.

We seem to map the math function by name to a builtin constant e.g. AsmJSMathBuiltin_sin. However that is not enough to know if that instance of the builtin should resist fingerprinting, we would need to look at the realm. Later we actually lookup the property and verify that it is the expected math function in ValidateMathBuiltinFunction. At that point we could maybe remember which version of the math builtin (native vs fdlibm) to use? That doesn't seem very clean for something that is supposed to verification however.

Of course we could also just fail the verification for sin/tan/cos that should resist fingerprinting, but that would cause unnecessary performance issues for users.

Comment 2

[Tom Schuster (MoCo)]

Hi Ryan! Could you take a look at this and help me figure out what the right approach could be?

(I really don't know how caching asm.js/wasm code works in details, that is why I am a bit hesitant about just modifying some existing data)

Comment 3

[Ryan Hunt [:rhunt]]

Hmm, this is tricky.

So we parse asm.js code and convert it to a wasm binary with some special features. We convert suspected Math builtin calls to special internal opcodes (e.g. MozOp::F64Sin) which we compile with Ion into builtin calls to these functions [1]. Then when we try to instantiate the asm.js code, we double check that the code actually imported the right Math builtins from the global, and not something else. If this check fails, we fallback to normal JS execution.

I think there are two paths here:

1. Add a 'isResistingFingerprinting' bool to JS::ReadOnlyCompileOptions, then emit new MozOp::RFP_F64Sin opcodes when the bool is set on parser.options(). These opcodes would call new builtin calls (e.g. SymbolicAddress:RFP_F64Sin) that would use fdlibm methods.
2. Or cause link fails when validating a math builtin that should resist fingerprinting and fall back to JS execution.

I think (2) would be pretty easy to implement, but would probably disable all asm.js content for resist fingerprinting users. We actually sort of want to remove asm.js support eventually, most people should be using wasm now. But not everyone is yet, so we haven't done it yet.

Option 1, is more involved but is mostly just plumbing things through.

[1] https://searchfox.org/mozilla-central/rev/31f5847a4494b3646edabbdd7ea39cb88509afe2/js/src/wasm/WasmBuiltins.cpp#1104-1160

Comment 4

[Tom Schuster (MoCo)]

Thank you Ryan! Seems like it's indeed not so simple.

1. I don't think this actually works. If I am understanding you correctly we would set the flag based on the asm.js function we are compiling? The relevant flag is actually on the realm of the called sin/cos/tan function instead.

I actually think your idea 1. could basically be combined with 2. though. Most of the time the resist fingerprinting flag on the asm.js function would match that of the math function, because they are in the same realm. So we simply remember the flag as you suggested in the compile options, but reject the math function call in the very unlikely case that the RFP flag is different.

Comment 5

[Ryan Hunt [:rhunt]]

Oh yeah, I forgot that you could have a Math function from a different realm than the module. Yeah, I'd guess that combining the two approaches would solve that then. Hopefully this is not too much work to add support for, we generally have de-prioritized asm.js.

Comment 6

[Tom Schuster (MoCo)]

Attachment: Bug 1827576 - Add shouldResistFingerprinting_ to CompileOptions. r?arai

I am not sure if we need to manually set the flag somewhere else, it's seems like realm() could be null

Comment 7

[Tom Schuster (MoCo)]

Attachment: Bug 1827576 - Respect resist-fingerprinting for sin/cos/tan in asm.js. r?rhunt

Depends on D176142

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2303675bbcc1
Add shouldResistFingerprinting_ to CompileOptions. r=arai
https://hg.mozilla.org/integration/autoland/rev/dda81b076dd1
Respect resist-fingerprinting for sin/cos/tan in asm.js. r=rhunt

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/2303675bbcc1
https://hg.mozilla.org/mozilla-central/rev/dda81b076dd1