Multisig vulnerability

[koinov]

I just checked your multisig contract and I really believe it is vulnerable. You accept incoming order if just one signature is valid. I believe this is a big mistake since one compromised private key can lead to draining funds from smart contract by submitting multiple external transactions with new orders.

I believe right solution is not to accept messages signed by less than predefined number of signatures or use internal messages sent through participants wallets.

This problem is described in my contest submission at https://contest.com/blockchain/entry390 in readme.txt within /multisig folder.

[ton-blockchain]

Thank you.

This is a known problem of the current implementation. We have plans to mitigate this and some other issues eventually.

[koinov]

Since this issue is still unsolved I tried to enforce your wallet, that is very well done and optimized, and added minimum required K parameter for every new request, so new request message is accepted only when number of valid signatures reaches this value.

You can check https://github.com/koinov/ton-multisig-enforced and particularly this commit

I also added FIFT scripts and some tests.

If you think this way is meaningful, I will be happy to do more tests, work on scripts and I also believe CLI is necessary for better MultiSig interaction.

[koinov]

Here's the another, and I believe more elegant solution that eliminate the possibility of draining all the funds from original multisig contract when one private key is compromised.

I've added unconfirmed orders limit parameter to the smart contract, so when number of new orders reaches the limit all new orders signed by the private key are reverted. This counter is reset as soon as any order is confirmed by at least K number of signatures.

This solution allows creating orders with the single key, that is really great, so no off-chain interactions between parties is required.

You can check https://github.com/koinov/ton-multisig-enforced-limit and particularly this commit

I believe this is the only safe and efficient way of operating multisig wallet using external messages.

[rainydio]

Hey, Dolphin here. This is my approach https://github.com/rainydio/ton-tolerant-personal-multisig/blob/master/code.fc

I started with assumption that multisig is used by a single company (e.g. exchange), where several clients are signing the same stream of orders with different keys. Orders are executed in-order (controlled by seqno), but may be accepted out of order (controlled by tolerance parameter). Once queue is filled no new orders are accepted. If mistake was made or malicious key filled the queue it's possible to sent reset vote, which wipes the queue out.

I also was worried that malicious key might blow the pending orders dictionary to some extreme size preventing legitimate message from ever reaching accept_message. Hence reset is a short-cut, with small and constant gas usage.

One more thing, I haven't seen used anywhere. Instead of having separate initialization code branch I ACCEPT constructor message and swap the code with actual contract code. <{ SETCP0 ACCEPT "code.fif" include PUSHREF SETCODE }>c is my stateinit.

Feel free to incorporate any of those ideas into your implementation @koinov. Also have look at test.fif, I don't have many test, but I made a simple suite which checks gas usage.

[koinov]

Hi man, nice seeing you here, your public testing was highly appreciated! We definitely need to find ideal balance between usability, security and optimisation. I believe original TON implementation has great functionality, perfect code and the only problem was this issue with malicious or compromised co-owner. I believe last fix is rather good solution.

What about the size of the message, I believe there is a TODO inside the contract limiting gas consumption, so I'm sure this will be fixed very soon on TVM level.

[koinov]

Updated multisig is looking good!