search

near / mpc

31 stars 6 forks source link

Security Issue - MPC recovery smart contract initializer can be initialized multiple times #471

Closed timurguvenkaya closed 3 months ago

timurguvenkaya commented 4 months ago

Asked for permission to create an Issue since SC is not ready yet

Description

Location: https://github.com/near/mpc-recovery/blob/develop/contract/src/lib.rs#L57

Description: While browsing the mpc contract to learn more about chain signatures & mpc as part of Guvenkaya (https://www.guvenkaya.co/) research, I saw a security vulnerability in the init function

ignore_state is used. It means you can initialize the contract multiple times.

In this scenario, by calling init, you can

  1. You can set a new threshold and set new candidates
  2. By constantly calling init, you can always keep the contract in the ProtocolContractState::Initializing state, making any functions which require different states non-callable

You use the same pattern in clean(), but you also have a private macro on top, which does not allow anyone to call it apart from the current account id

Recommendation

Consider either adding #[private] macro or not using ignore_state

volovyks commented 4 months ago

Thank you @timurguvenkaya! Adding this to our board.

volovyks commented 3 months ago

Fixed, thanks you @timurguvenkaya.