search

code-423n4 / 2022-04-mimo-findings

0 stars 0 forks source link

Generic ReEntrancy in DemandMinerV2 deposit and withdraw #12

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/DemandMinerV2.sol#L67 https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/liquidityMining/v2/DemandMinerV2.sol#L90

Vulnerability details

Impact

The functions deposit and withdraw in DemanMinerV2 are not conformant to CEI (Checks Effects Ineractions) as they transfer the token (external call) before performing internal checks and updates.

For this reason, if the contract was setup to use a token with hooks (e.g. ERC777) reEntrancy could be exploited, creating undefined scenarios

Recommended Mitigation Steps

Add nonReentrant modifier to these functions

m19 commented 2 years ago

While the recommended CEI pattern isn't being followed a reentrancy attack is not actually possible, even with an ERC777 token. A reentrancy attack on the withdraw function would only work if the transfer happens between the check and the state update. We also don't see any way a reentrancy on the deposit function is possible.

gzeoneth commented 2 years ago

Reentrancy without exploit. Downgrading to Low / QA.

gzeoneth commented 2 years ago

Treating as warden's QA report.