code-423n4 / 2021-05-visorfinance-findings

0 stars 0 forks source link

Delegated transfer of owner fails #47

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Vulnerability Details

The Visor.delegatedTransferERC20 function skips the approval check if msg.sender == _getOwner(), however, it will still try to reduce the approval in that case. As it is implemented that the owner does not need an approval for this function, a previous approve action was most likely never sent and the transaction will fail when trying to reduce the erc20Approvals field by the amount due to the usage of SafeMath underflow checks.

Impact

Owners cannot transfer using this function.

Recommended Mitigation Steps

Move the approval subtraction to the if path.

ghost commented 3 years ago

sponsor acknowledged disagree with severity 0 While this is true, delegatedTransferERC20 isnt meant for owner but suggestion is noted.

ghoul-sol commented 3 years ago

Duplicate of #21