search

code-423n4 / 2022-05-backd-findings

0 stars 0 forks source link

RewardHandler.burnFees() will work only once and revert after that if burnedAmount is different. #88

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-backd/tree/main/protocol/contracts/RewardHandler.sol#L52

Vulnerability details

Impact

RewardHandler.burnFees() will work only once and revert after that if burnedAmount is different.

Proof of Concept

OpenZeppelin’s safeApprove() will revert if the account already is approved and the new safeApprove() is done with a non-zero value. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/fcf35e5722847f5eadaaee052968a8a54d03622a/contracts/token/ERC20/utils/SafeERC20.sol#L45-L58

Tools Used

Solidity Visual Developer of VSCode

Recommended Mitigation Steps

They must first be approved by zero and then the actual allowance must be approved. Or you can use safeIncreaseAllowance() instead.

chase-manning commented 2 years ago

It becomes zero again after the function is called because we use the approval amount. The function does not only work once.

GalloDaSballo commented 2 years ago

In lack of any coded POC, considering that the depositFees function does a transferFrom for the full amount

Screenshot 2022-06-21 at 03 49 52

I believe the finding to be invalid