code-423n4 / 2024-03-ondo-finance-findings

5 stars 6 forks source link

transferFrom uses allowance even if _sender == _recipient #248

Closed c4-bot-10 closed 6 months ago

c4-bot-10 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L281-L286

Vulnerability details

Impact

rOUSG#transferFrom attempts to use allowance even when _sender = _recipient. This breaks compatibility with a large number of protocol who opt to use the transferFrom method all the time (pull only) instead of using both transfer and transferFrom (push and pull). The ERC20 standard only does an allowance check when spender != from. The result of this difference will likely result in tokens becoming irreversibly stranded across different protocols.

Proof of Concept

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L281-L286 The trasnferFrom method shown above always uses allowance even if _sender = _recipient.

Token won't be compatible with some protocols and will end up stranded

Tools Used

Manual Review

Recommended Mitigation Steps

Only use allowance when _sender != _recipient

Assessed type

ERC20

0xRobocop commented 6 months ago

Invalid

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/8a7a9c585706503f5250f09b033c94cf7ee37db2/contracts/token/ERC20/ERC20.sol#L301

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as insufficient quality report

3docSec commented 6 months ago

Inconsequential (and common practice as noted in presort)

c4-judge commented 6 months ago

3docSec marked the issue as unsatisfactory: Invalid