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

5 stars 6 forks source link

"multiexcall()" does not have reentrancy protection leading to DoS #3

Closed c4-bot-3 closed 7 months ago

c4-bot-3 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/be2e9ebca6fca460c5b0253970ab280701a15ca1/contracts/ousg/rOUSGFactory.sol#L121

Vulnerability details

Impact

"multiexcall()" function inside of "ROUSGFactory.sol" does not have reentrancy protection and while doing the batch call reentrancy is possible, even though there is no state changes and funds at risk, it can introduce excessive calls by triggering the function repeatedly, and therefore introduce DoS (Denial of Service) by consuming gas and slowing down the contract’s execution, and potentially transaction running out of gas therefore reverting.

Proof of Concept

contract Attack {
    ROUSGFactory public factory;

    constructor(address _factory) {
        factory= ROUSGFactory(_factory);
    }

    // Fallback is called when ROUSGFactory sends native token to this contract 
    // via call and adding value attribute
    fallback() external payable {
        factory.multiexcall(array full of dummy data);
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

Add reentrancy mechanism to the function, one solution would be:

    bool internal locked;

    modifier noReentrant() {
        require(!locked, "No re-entrancy allowed!");
        locked = true;
        _;
        locked = false;
    }

Assessed type

DoS

c4-pre-sort commented 7 months ago

0xRobocop marked the issue as insufficient quality report

3docSec commented 7 months ago

multiexcall has access control in place, so no malicious actor can call it, let alone re-enter it

c4-judge commented 7 months ago

3docSec marked the issue as unsatisfactory: Invalid