Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 206 forks source link

When deploying a malicious contract, it is possible to crash the Zoe vat #4276

Closed b4d2 closed 2 years ago

b4d2 commented 2 years ago

Describe the bug

By creating a zcfMint with an extremely large token name, after installing the Dapp in the wallet, the zoe vat crashes

To Reproduce

  1. Create a new Dapp with a contract (see the getting starting guide)
  2. Wait for setup to finish
  3. Modify the makeZCFMint with the following code
    const tokenName = 'A'.repeat(200000000);
    const zcfMint = await zcf.makeZCFMint(tokenName, AssetKind.NAT, harden({ decimalPlaces: 3 }));
  4. Deploy the contract code (agoric deploy ./contract/deploy.js ./api/deploy.js) contract code will install on zoe,
2022-01-11T19:27:52.916Z SwingSet: kernel: delivery problem, terminating vat v11 Allocate meter exceeded

Deploy Output

% agoric deploy ./contract/deploy.js ./api/deploy.js

Open CapTP connection to ws://127.0.0.1:8000/private/captp...o
agoric: deploy: running /Users/b4d2/dapp/demo/contract/deploy.js
agoric: deploy: Deploy script will run with Node.js ESM
- SUCCESS! contract code installed on Zoe
-- Contract Name: fungibleFaucet
-- Installation Board Id: 1221243446
writing /Users/b4d2/dapp/demo/ui/public/conf/installationConstants.js
agoric: deploy: running /Users/b4d2/dapp/demo/api/deploy.js
agoric: deploy: Deploy script will run with Node.js ESM
CapTP bundle exception: (RemoteError(error:captp:unknown#20001)#1)
RemoteError(error:captp:unknown#20001)#1: vat terminated

  at fullRevive (packages/marshal/src/marshal.js:399:34)
  at unserialize (packages/marshal/src/marshal.js:482:19)
  at CTP_RETURN (packages/captp/src/captp.js:504:16)
  at dispatch (packages/captp/src/captp.js:568:9)
  at WebSocket.<anonymous> (packages/agoric-cli/src/deploy.js:117:13)
  at WebSocket.emit (node:events:390:28)
  at WebSocket.emit (node:domain:475:12)
  at Receiver.emit (node:events:390:28)
  at Receiver.emit (node:domain:475:12)
  at Socket.emit (node:events:390:28)
  at Socket.emit (node:domain:475:12)

RemoteError(error:captp:unknown#20001)#1 ERROR_NOTE: Rejection from: (Error#2) : 19 . 0
Nested error under RemoteError(error:captp:unknown#20001)#1
  Error#2: Event: 18.1

    at deployApi (file:///Users/b4d2/dapp/demo/api/deploy.js:88:64)
    at async WebSocket.<anonymous> (packages/agoric-cli/src/deploy.js:264:13)

agoric: (RemoteError(error:captp:unknown#20001)#1)
  1. Attempt to send 1 RUN from "Zoe fees" to Transfer within wallet "Agoric RUN currency", and the transfer will fail since zoe is dead Screen Shot 2022-01-11 at 11 31 12 AM

Expected behavior

I should not be able to crash the zoe vat from a malicious Dapp

Additional notes

This may result in a cascading failure as well, as the Token name is utilized by multiple services downstream from zoe

Platform Environment

ref #4264

warner commented 2 years ago

Ok, so a 200MB string is created by the deploy script, then sent through various Zoe APIs until a vat is terminated. Things I plan to investigate:

Wen we figure out fees, it sounds like this message should have cost too much to deliver, which would be an extra layer of protection on top of whatever else we come up with.

Two more general questions:

warner commented 2 years ago

In today's discussion, we figured that a 1MB hard-coded limit would be good, and means that we're confident that a single 1MB message won't crash anything. And we'll state that we expect all messages to be less than 10kB in size, so we'll apply a nominal fee to anything smaller than that, and grow it (possibly super-linearly) beyond that point. The intention is that the fee starts to become painfully expensive well before we get to the hard-coded limit. All of these limits and fee curves should be configurable through governance so we can change them later without a deep upgrade.

Once the #3269 bundle-install pathway is done, contract code will no longer be carried in messages (either into the chain or between vats, although the bundle will still be in the response to the D(bundleCap).getBundle() device-node invocation syscall that ZCF does). That will remove the most pressing use case for 2MB+ messages. We don't really want large computation to be performed on-chain (so we don't want to support a use-case of e.g. processing large bitmaps), but we can still imagine a single operation that interacts with e.g. hundreds of other objects, and which might need to point at all of them, so 10kB may not be enough. We don't yet know where to draw the line. We currently feel pretty comfortable allowing small messages, and pretty nervous about allowing large messages, but as we discover new attacks and use cases, those feelings will change. Having a variable fee will apply an incentive to express our anti-desire for large computation, and hopefully the hard limit will provide some protection against a useful number of attacks.