code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Lock.sol#L104

Vulnerability details

Impact

Release function in Lock.sol used the .transfer parameter in last line. That means totalLocked[asset] -= lockAmount; updated before transfer. So when .transfer failed, user lost his fund. .transfer only provides 2300 gas for its operation. This means the following cases can cause the transfer to fail:

The contract does not have a payable callback

The contract’s payable callback spends more than 2300 gas (which is only enough to emit something)

The contract is called through a proxy which itself uses up the 2300 gas

Proof of Concept

   function release(
        uint _id
    ) public {
        claimGovFees();
        (uint amount, uint lockAmount, address asset, address _owner) = bondNFT.release(_id, msg.sender);
        totalLocked[asset] -= lockAmount;
        IERC20(asset).transfer(_owner, amount);
    }

https://code4rena.com/reports/2022-07-fractional/#m-11-use-of-payabletransfer-may-lock-user-funds