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

5 stars 6 forks source link

Unlimited Wrapping of OUSG #291

Closed c4-bot-6 closed 6 months ago

c4-bot-6 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/78779c30bebfd46e6f416b03066c55d587e8b30b/contracts/ousg/rOUSG.sol#L415-L422

Vulnerability details

Impact

In ousgInstantManager, the function mintRebasingOUSG allows minting of rOUSG. The function handles the approval of OUSG to the rOUSG contract for the 'ousgAmountOut'. It then calls the rOSUG.wrap() function. This function calculates the appropriate amount of shares, mint the shares to the msg.sender, and then transfers the OUSG from the user to the rOUSG contract.

The issue lies in the fact that the rOUSG.wrap() can be called externally and doesn't validate the approval of _OUSGAmount nor is the result of ousg.transferFrom checked to be true. As a result any amount can be given directly to the rOUSG.wrap() function which results in the shares being minted and the transferFrom silently failing.

Proof of Concept

Reference Links to affected code

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a check on allowance of ousg. Reference: https://github.com/code-423n4/2024-03-ondo-finance/blob/78779c30bebfd46e6f416b03066c55d587e8b30b/contracts/ousg/ousgInstantManager.sol#L298-L301

Assessed type

Invalid Validation

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as insufficient quality report

3docSec commented 6 months ago

Invalid: transferFrom will revert if the caller hasn't approved the contract to use enough tokens. Also, wrap takes tokens from msg.sender so dangling approvals can't be exploited.

c4-judge commented 6 months ago

3docSec marked the issue as unsatisfactory: Invalid