Lack of safeApprove(0) may prevent deposits and swapping of token

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/FeeBurner.sol#L72 https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/FeeBurner.sol#L107 https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/RewardHandler.sol#L46 https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/RewardHandler.sol#L52

Vulnerability details

Impact

OpenZeppelin’s safeApprove() will revert if the account already is approved and the new safeApprove() is done with a non-zero value.

Users may be prevented from swapping tokens to Backd LPTokens a second time

Proof of Concept

https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/FeeBurner.sol#L72 https://github.com/code-423n4/2022-05-backd/blob/main/protocol/contracts/tokenomics/FeeBurner.sol#L107

There are multiple places safeApprove() is called a second time without setting the value to zero first.

Tools Used

Manual review

Recommended Mitigation Steps

Use safeApprove(0) if the allowance is being changed, or use safeIncreaseAllowance()

[GalloDaSballo]

The warden links to a function that will do nothing if the approval is non-zero

For that reason I believe the finding to be invalid as either the approval is max, or is zero