code-423n4 / 2023-01-ondo-findings

0 stars 0 forks source link

STRICT EQUAL CHECK OF UINT VALUES COULD LEAD TO REVERT BY FRONT RUNNING THE TRANSACTION #367

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/cash/CashManager.sol#L336-L350

Vulnerability details

Impact

In the function setPendingMintBalance of the CashManager.sol there is a strict uint256 not equal (!=) check inside an if statement, between oldBalance and mintRequestsPerEpoch[epoch][user]. And this function is given the role of MANAGER_ADMIN. So if the MANAGER_ADMIN decides to decrease the balance of user, then user can front run this transaction by calling requestMint() function and changing his mintRequestsPerEpoch[currentEpoch][msg.sender] value. Since the value of mintRequestsPerEpoch[currentEpoch][msg.sender] changes for the user it will no longer be equal to the oldBalance in setPendingMintBalance function. Hence the transaction will revert with UnexpectedMintBalance() error code.

Proof of Concept

if (oldBalance != mintRequestsPerEpoch[epoch][user]) {
  revert UnexpectedMintBalance();
}

Above if condition can be made false by changing the mintRequestsPerEpoch[epoch][user] value, if the MANAGER_ADMIN decides to decrease the balance of the user. User can do this by calling the requestMint function and changing the mintRequestsPerEpoch[currentEpoch][msg.sender] using below line of code.

mintRequestsPerEpoch[currentEpoch][msg.sender] += depositValueAfterFees;

Hence the user can claim more token balance before the MANAGER_ADMIN can change the balance amount using the setPendingMintBalance command.

Tools Used

Manual Review.

Recommended Mitigation Steps

Should prevent user from calling the requestMint function while setPendingMintBalance function is being called by the MANAGER_ADMIN. This can be done by using boolean values and changing thier state accordingly when setPendingMintBalance function is called.

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2023-01-ondo-findings/issues/335

trust1995 commented 1 year ago

This is more of a property of blockchains rather than a vulnerability.

c4-judge commented 1 year ago

trust1995 marked the issue as grade-b

c4-judge commented 1 year ago

trust1995 marked the issue as grade-a