Incorrect hash results for scrypt when WASM is disabled

[quexten]

Hi,

scrypt seems to deliver incorrect results when WASM is disabled. To reproduce, here is the test code:

import { scryptAsync } from "@noble/hashes/scrypt";
import sodium from "libsodium-wrappers-sumo";

(async() => {
    var password = new TextEncoder("utf-8").encode("abc");
    var salt = new TextEncoder("utf-8").encode("123");
    const passwordBuf = new Uint8Array(password);
    const saltBuf = new Uint8Array(salt);
    await sodium.ready

    var sodiumHash = sodium.crypto_pwhash_scryptsalsa208sha256_ll(passwordBuf, saltBuf, 2**16, 8, 1, 32, "uint8array")
    var nobleHash =  await scryptAsync(passwordBuf, saltBuf, { N: 2**16, r: 8, p: 1, dkLen: 32 });

    console.log("Sodium: " + Buffer.from(sodiumHash).toString("hex"))
    console.log("Noble-Hashes: " + Buffer.from(nobleHash).toString("hex"))
})()

When run with WASM enabled:

node index.js                                                                                                           
Sodium: d5001a65d108a11101286f0e6723adf04c0b0130a09897d9b3a2a8dad28e123b
Noble-Hashes: d5001a65d108a11101286f0e6723adf04c0b0130a09897d9b3a2a8dad28e123b

With WASM disabled:

node --noexpose_wasm index.js                                                                                              
Sodium: 387fece14f9bcf4e394cbf35402277079b49a22dd54a57a1400000002a000000
Noble-Hashes: d5001a65d108a11101286f0e6723adf04c0b0130a09897d9b3a2a8dad28e123b

[jedisct1]

Do you get the same output every time?

With node --noexpose_wasm I actually get:

Aborted(no native wasm support detected)

So the function call probably doesn't complete and what I get is uninitialized data.

[jedisct1]

The javascript and wasm versions use the same code and are expected to behave the same way.

Maybe bug in emscripten?

The binding for that function has been removed until someone can possibly figure it out.

[quexten]

Do you get the same output every time?

With node --noexpose_wasm I actually get:

Aborted(no native wasm support detected)

So the function call probably doesn't complete and what I get is uninitialized data.

Ah, interesting I do indeed have non-deterministic output. Here are some runs:

~/p/testscrypt ❱ node --noexpose_wasm index.js                                                                                               16:11:16
Sodium: afd177f7bda99b3f9f2bc4ae6636932e7f9d895dfb0b945a400000002a000000
Noble-Hashes: d5001a65d108a11101286f0e6723adf04c0b0130a09897d9b3a2a8dad28e123b
~/p/testscrypt 1.7s ❱ node --noexpose_wasm index.js                                                                                          16:11:21
Sodium: 9bfb0ac8517848976c1a7685e7ef6a71381ad451730a49ee400000002a000000
Noble-Hashes: d5001a65d108a11101286f0e6723adf04c0b0130a09897d9b3a2a8dad28e123b
~/p/testscrypt 1.7s ❱ node --noexpose_wasm index.js                                                                                          16:11:24
Sodium: 18056becfac1e788c0c555660e40e4f7e822697ef1e1d8f8400000002a000000
Noble-Hashes: d5001a65d108a11101286f0e6723adf04c0b0130a09897d9b3a2a8dad28e123b
~/p/testscrypt 1.7s ❱ node --noexpose_wasm index.js                                                                                          16:11:26
Sodium: 010fdaeed0ad459325dea5099579d4f6470e1fad39a146ff400000002a000000
Noble-Hashes: d5001a65d108a11101286f0e6723adf04c0b0130a09897d9b3a2a8dad28e123b

I do not see the error message though.

[jedisct1]

The reason seems to be the values used for -s TOTAL_MEMORY during compilation: https://github.com/jedisct1/libsodium/blob/ebcc2a66366dc2f990f37e4114cf52ab4cedcba1/dist-build/emscripten.sh#L7-L9

With higher value, we get the correct results.

Their build script enables -s ALLOW_MEMORY_GROWTH=1 that presumably allows reserving a minimal amount of memory that can then automatically grow on demand.

But that doesn't seem to work. Maybe this is a WebAssembly-only thing and doesn't work in JavaScript.

Using a large value for TOTAL_MEMORY is a waste of memory for people not using these functions.

So maybe we should revert to the old (before -s ALLOW_MEMORY_GROWTH was added) behavior, which was to only enable the password hashing function in sumo builds, and reserve a lot of memory only in these builds as well.