code-423n4 / 2022-06-nibbl-findings

1 stars 0 forks source link

DoS with block gas limit--External calls inside a loop might lead to a denial-of-service attack. #319

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/Basket.sol#L68-L74

Vulnerability details

Impact --Check: calls-loop --Severity: Medium --Confidence: Medium External calls can fail accidentally or deliberately, which can cause a DoS condition in the contract.

Proof of Concept --https://swcregistry.io/docs/SWC-113 --ConsenSys Smart Contract Best Practices --https://fravoll.github.io/solidity-patterns/pull_over_push.html --https://ethereum-contract-security-techniques-and-tips.readthedocs.io/en/latest/recommendations/

Tools Used --slither

Recommended Mitigation Steps Favor pull over push strategy for external calls. To minimize the damage caused by such failures, it is better to isolate each external call into its own transaction that can be initiated by the recipient of the call. This is especially relevant for payments, where it is better to let users withdraw funds rather than push funds to them automatically (this also reduces the chance of problems with the gas limit).

Ex: Pull method

// This code has not been professionally audited, therefore I cannot make any promises about // safety or correctness. Use at own risk. contract PullOverPush {

mapping(address => uint) credits;

function allowForPull(address receiver, uint amount) private {
    credits[receiver] += amount;
}

function withdrawCredits() public {
    uint amount = credits[msg.sender];

    require(amount != 0);
    require(address(this).balance >= amount);

    credits[msg.sender] = 0;

    msg.sender.transfer(amount);
}

}

KenzoAgada commented 2 years ago

The function linked takes as an argument which tokens to withdraw

HardlyDifficult commented 2 years ago

The function linked takes as an argument which tokens to withdraw

Agree. I believe this report is not valid.

If one of the addresses attempts to DOS the call, the user could retry excluding the malicious address. Additionally the function here doesn't do anything other than make that external call.