search

safe-global / safe-wallet-web

Safe{Wallet} – multisig EVM wallet
https://app.safe.global
GNU General Public License v3.0
346 stars 408 forks source link

Check that newly added signers can actually sign #3463

Open SKYBITDev3 opened 6 months ago

SKYBITDev3 commented 6 months ago

There is a risk that when a multisignature Safe account is created, some signers whose addresses were added by the creator cannot actually sign (the Safe creator may not have known that at the time), which can result in stuck funds if funds are deposited before anyone realizes that the minimum number of signatures required for can't be reached for a transaction to be executable.

Let's go through an example. Someone starts to create a Safe account having 3 signers and requiring 2 signatures per transaction. Her own address is included as the first signer. She then adds 2 additional addresses of people she knows. On completion, the UI says that the account is now ready to use. She deposits 100k USDT into the Safe. But let's say one of the addresses was wrong, and the owner of the other address hadn't backed up his passphrase and lost the device that had his wallet. So neither of them can actually sign for any transactions. After realizing this, she tries to add another address, but to do so requires at least 2 signatures. So the 100k USDT is stuck.

So there needs to be ways to prevent this from happening.

Maybe add a final step during Safe creation that requires each listed signer to sign. Maybe display a warning if that hasn't yet been done, with a button that leads the user to do it.

SKYBITDev3 commented 6 months ago

Maybe there should also be a check that a recoverer can actually recover the account. Currently any address can be added as a recoverer with no verification.

katspaugh commented 6 months ago

Fair points! cc @kirkkonen

SKYBITDev3 commented 6 months ago

Another possibility is to add a requirement during the creation (or editing) of a Safe for a newly added signer to actually sign right then. This ensures that the new Safe contract is only created on-chain after it has been verified that all added addresses are of signers who can actually sign. This would prevent waste of gas in creating an unusable Safe contract.

SKYBITDev3 commented 6 months ago

Maybe there should also be a check that a recoverer can actually recover the account. Currently any address can be added as a recoverer with no verification.

Using a recoverer can be the last resort path to regain access to the stuck funds, so it could actually be even more important than the original issue. If it's only found much later (when the recoverer needs to be invoked) that there's no way for the recoverer to sign, then the funds would be truly stuck. So there should be a step that checks that the designated recoverer can actually sign.

SKYBITDev3 commented 6 months ago

There's also the possibility that one or more signers may no longer be able to sign any time in the future for whatever reason (e.g. death, insanity, lost device and backup etc.). So maye there should be regular (e.g. monthly) verification of signing ability by listed signers and recoverers - if one day a signer fails to sign during a regular check then a warning could be shown on the UI and a decision could be made by the others to replace it with an address of someone who can sign (which should also undergo verification by the platform).