Closed warner closed 2 years ago
I'm inclined to re-phrase this as a bug:
I don't yet have evidence that it is vulnerable, but I think the premise of this issue is that we presume that it is.
Is this truly a pso blocker if we set Gas sufficiently high?
we don't know. That is: it's a blocker until I/we do at least a little info gathering.
@dckc is going to enumerate the smart-wallet operations and the user-controlled parameters of each,
What's currently on master is:
approveOffer
suggestIssuer
The parameter to each is a string that gets JSON.parse'd and then approveOffer
...
E(zoe).offer(invitation, payments)
I don't have a good feel for exactly what happens in step 4.
I'm not exactly sure about the flow here. I know that there was a problem where payment records grew with each update (fixed in a pending PR).
If a smart-wallet user chooses to store really large petnames ...
That's not an option in the current API.
Somewhat careful study of the walletFactory (aka smartWallet) contract shows that storage and compute costs are accounted for.
I suppose the next step is to turn this over to the kernel team to measure experimentally. @Tartuffo I'm routing this thru you.
cc @turadg
We charge 10 BLD for this, and it does O(1) work and allocation (provided makeScalarBigMapStore
gets and sets count as O(1)).
We charge BeansPerMessage
and BeansPerMessageByte
for this (#6049). These are under governance control, so provided the cost is linear, which it seems to be, we can set an appropriate price.
We validate against this shape:
WalletBridgeMsg: M.split(
{
owner: M.string(),
type: M.string(),
blockHeight: M.number(),
blockTime: M.number(),
},
M.or({ action: M.string() }, { spendAction: M.string() }),
),
It does return a Promise
to the bridge manager in the bootstrap vat.
The bootstrap vat drops the promise, so perhaps we should drop the promise inside the walletFactory vat and return void rather than returning the promise?
void E(srcHandlers.get(srcID)).fromBridge(srcID, obj);
we E(publicMarshaller).unserialize(actionCapData)
and dispatch to a facet by method. The only facet and method currently exposed is offers.executeOffer(action.offer)
guarded as executeOffer: M.call(shape.OfferSpec).returns(M.promise()),
.
The promise is returned to handleBridgeAction and hence to fromBridge.
I don't see any loops in the code paths of executeOffer
except over the fields in a PaymentRecord (a PSM trade has 1 field in such records).
As an offer progresses (a seat allocated, offerResult available, wants satisfied) the status is published, along with give / want / payouts records, to chainStorage. It seems to be about 6 updates per PSM trade, each of 400 bytes or less: https://github.com/Agoric/agoric-sdk/issues/6038#issuecomment-1242631519
This only applies to voting, not to PSM trades. And only gov cttee members have invitations that give them access to start the voting process.
Each time an asset is added the vbank, a notification goes out to all wallets, and they all add a purse. The power to add vbank assets is reserved to the BLDer DAO.
The bridge vat drops the promise, so perhaps we should drop the promise inside the walletFactory vat and return void rather than paying for an inter-vat promise?
cc @turadg
perhaps we should drop the promise inside the walletFactory vat and return void rather than paying for an inter-vat promise?
TIL: returning void won't save the inter-vat promise. To do that requires something more like E.sendOnly(srcHandlers.get(srcID)).fromBridge(srcID, obj)
which is not fully baked.
This is satisfactory for PSM-First.
What is the Problem Being Solved?
In today's Wallet meeting, we were talking about storage-consumption attacks against the smart wallet vat. Each provisioned smart-wallet account can send signed instruction messages (via the bridge device) into this vat, and the code there will interpret those instructions to do things like initiate transfers, assign petnames, and create/accept offers. The smart wallet retains some amount of data for each account: purse/payment references, petname assignments, open offers, etc. (We keep these on the chain side specifically so that the user doesn't need to remember anything more than their seed phrase).
We'd like to know how much storage space each one of these options might incur, to reason about how much it would cost an attacker to try and exhaust this storage. Once the smart wallet (and the vats it depends upon) gets fully virtualized (#4489), we don't think RAM usage should be a problem, but disk/DB usage is still a consideration. For example, each user has a table of petnames that map strings to/from object references like Issuers. This is probably stored as a pair of BigDurableMapStores, with string keys (and Presence/vref values) and vice versa. Adding one entry to a virtual/durable store like this costs one entry in the LMDB database, whose DB key is a serialized representation of the string key (this is cheap, we merely prefix an
s
), and whose value is the capdata serialization of the target's vref (something like{"body":"{\\"@qclass\\":\\"slot\\",\\"iface\\":\\"Alleged: setStore\\",\\"index\\":0}","slots":["o+8/11"]}
). LMDB has some overhead, but the data we feed into the DB for a petname of length L would then be about1+L+96
.If a smart-wallet user chooses to store really large petnames, or a lot of them, they could drive the smart-wallet vat to use a lot of DB space, which then adds to the disk requirements on all validators and followers. In the long run, we want the cost of this to be captured in a meter and charged to the user. But in the short term, we just need to make sure this can't cause problems.
The task is to
I can think of two ways to measure this:
.agoric/data/ag-cosmos-chain-state/data.mdb
) as we submit hundreds or maybe thousands of the same operation, to measure the slope of DB size vs num_operationsdata.mdb
file might be sparse and pre-allocated to be pretty large, so might need to look at the actual used blocks (not just whatls -l
reports), or e.g. compress it and use the compressed size as a proxy for the actual consumptionkey.length + value.length
and accumulate it across all add/modify/delete DB operationsLMDB might have some sort of API to ask about total used space too, which would be the best option. I see http://www.lmdb.tech/doc/group__mdb.html#structMDB__stat , maybe the "pages" it counts are used for both keys and values. It might be good to enhance
@agoric/swing-store
to add agetStats()
method, and to include this number in the telemetry metrics we report out of the chain (and into tools like honeycomb).In either case, we'd need to build a test scaffold that can submit N copies of the operation under test, and measure the DB size every once in a while, and build up a graph to correlate them.
@dckc is going to enumerate the smart-wallet operations and the user-controlled parameters of eah, so we can figure out what our experiment needs to test.
Description of the Design
Security Considerations
Test Plan