search

code-423n4 / 2022-07-fractional-findings

0 stars 0 forks source link

Migration Module: Disable to join, leave, commit by starting a buyout #570

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/modules/Migration.sol#L116-L118 https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/modules/Migration.sol#L148-L150 https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/modules/Migration.sol#L189-L191

Vulnerability details

Impact

MED - the function of the protocol could be impacted. Anyone can call Buyout::start to disable to join, leave, commit functions for migration proposal

Proof of Concept

The Buyout module is unaware of migration module. So, even when some migration is proposed, anybody can start a buyout process. On the other hand, one cannot use propose, leave, join, commit, withdrawContribution from Migration module when a buyout for the relevant vault is active. Using this, a malicious actor can start buyout process just to prevent people from using these functions. For example, somebody proposed a migration, and an attacker starts a buyout so that none can join. For another example, somebody proposed a migration and sent some eth with join function. Then an attacker starts a buyout. As the result, the eth is locked during the buyout period. The attacker can call end and start to keep the asset locked.

// modules/Migration.sol::join
// The buyout state is checked

115         // Reverts if buyout state is not inactive
116         (, , State current, , , ) = IBuyout(buyout).buyoutInfo(_vault);
117         State required = State.INACTIVE;
118         if (current != required) revert IBuyout.InvalidState(required, current);

Tools Used

none

Recommended Mitigation Steps

One idea for mitigation would be to prevent to start buyout when there is a proposal. But this weakens the modularity and it can be used to disable the buyout functionality. Another idea would be not checking the buyout state for functions like join, leave. However, it may open up some attack vectors.

stevennevins commented 2 years ago

Duplicate of https://github.com/code-423n4/2022-07-fractional-findings/issues/123

HardlyDifficult commented 2 years ago

Duping to https://github.com/code-423n4/2022-07-fractional-findings/issues/230