Bug: Candid deserialization of large arrays fails with buffer overflow on read

[crusso]

This was hard to reproduce locally using drun (perhaps because of my VM's resources) and annoying to repro with dfx because it refused to upgrade to code with the same hash.

But the playground was just right:

https://m7sm4-2iaaa-aaaab-qabra-cai.raw.ic0.app/?tag=336007141

Consider the code:

import Array "mo:base/Array";
import Debug "mo:base/Debug";

actor {
  stable var a : [var Nat] =
    Array.init(268435456 / 2, 0x04); // ca. 0.5GB array
  system func preupgrade() { Debug.print(debug_show({pre=a.size()})); };
  system func postupgrade() { Debug.print(debug_show({post=a.size()})); }
}

Deploying and then doing a self-upgrade produces the following crash (see the playground log)

[4:42:56 PM] moc version 0.6.17
[4:42:56 PM] base library version dfx-0.8.5-beta.0
[4:43:09 PM] Compiling code...
[4:43:10 PM] Compiled Wasm size: 168KB
[4:43:10 PM] Deploying code...
[4:43:10 PM] Requesting a new canister id...
[4:43:19 PM] Got canister id 4r7kv-yiaaa-aaaab-qac5a-cai
[4:43:29 PM] Code installed at canister id 4r7kv-yiaaa-aaaab-qac5a-cai
[4:43:37 PM] Compiling code...
[4:43:38 PM] Compiled Wasm size: 168KB
[4:43:38 PM] Deploying code...
[4:44:01 PM] Call was rejected: Request ID: 02b62063686f0338440239d22696a69e5be3f1833d36ca63a51b1423deaf0134 Reject code: � Reject text: Canister 4r7kv-yiaaa-aaaab-qac5a-cai trapped explicitly: IDL error: byte read out of buffer

I first noticed the bug locally on master (0.6.20), so the outdated moc version of the playground is not the culprit.

This seems to be the line that is trapping:

motoko/rts/motoko-rts/src/buf.rs

Line 23 in 91df5fd

idl_trap_with("byte read out of buffer");

Perhaps an error decoding the larger array size and payload range?

UPDATE: Got a drun repro #3067 (by decreasing array size)

[nomeata]

Could you bisect the failing array length? Knowing the shortest array where it fails may give us a good lead.

[crusso]

Nope, not yet. My suspicion is that our Rust leb128 encoders might be dodgy, but it's just a hunch.

[crusso]

I think the leb decoders are ok, and am now suspecting that we don't check for overflow when pre-computing the size of the serialized blob during serialization. I can't see an overflow check in the code. @nomeata, seem plausible?

[crusso]

FTR, the problem was an missing overflow check in alloc_word... Be afraid, be very afraid.