sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

John_Femi - No check between Avail token and ERC20 Token #128

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

John_Femi

medium

No check between Avail token and ERC20 Token

Summary

Value or message can be sent from sendERC20 function and redeemed with the receiveAvail function due to no necessary checks in the receiveAvail fiction. This can lead to a massive loss for the protocol where a large amount of avail is minted in place for the same amount in ERC20 token but a lower value against usdt as AVAIL/USDT !== ERC20/USDT.

Vulnerability Detail

Looking at this test function

function test_sendEC20_ReceiveAvail(uint256 amount, bytes32 rangeHash , bytes32 assetId) external {
        address tod = makeAddr("to");
        bytes32 to = bytes32(bytes20(tod));
        vm.assume(to != bytes32(0) && amount != 0);
        address from = makeAddr("from");
        ERC20Mock token = new ERC20Mock();
        token.mint(from, amount);
        bytes32[] memory assetIdArr = new bytes32[](1);
        assetIdArr[0] = assetId;
        address[] memory tokenArr = new address[](1);
        tokenArr[0] = address(token);
        vm.prank(owner);
        bridge.updateTokens(assetIdArr, tokenArr);
        IAvailBridge.Message memory message =
            IAvailBridge.Message(0x02, bytes32(bytes20(from)), to, 2, 1, abi.encode(assetId, amount), 0);
        vm.startPrank(from);
        token.approve(address(bridge), amount);
        vm.expectCall(address(token), abi.encodeCall(token.transferFrom, (from, address(bridge), amount)));
        bridge.sendERC20(assetId, to, amount);
        assertEq(bridge.isSent(0), keccak256(abi.encode(message)));
        assertEq(token.balanceOf(from), 0);
        assertEq(token.balanceOf(address(bridge)), amount);

        vm.assume(amount != 0);
        //address to = makeAddr("to");
        message =
            IAvailBridge.Message(0x02, bytes32(bytes20(from)), to, 1, 2, abi.encode(assetId, amount), 0);
        bytes32 messageHash = keccak256(abi.encode(message));
        bytes32 dataRoot = keccak256(abi.encode(bytes32(0), messageHash));

        vectorx.set(rangeHash, dataRoot);

        bytes32[] memory emptyArr;
        IAvailBridge.MerkleProofInput memory input =
            IAvailBridge.MerkleProofInput(emptyArr, emptyArr, rangeHash, 0, bytes32(0), messageHash, messageHash, 0);

        vm.expectCall(address(avail), abi.encodeCall(avail.mint, (tod, amount)));
        bridge.receiveAVAIL(message, input);
        assertTrue(bridge.isBridged(messageHash));
        assertEq(avail.totalSupply(), amount);
    }

Based from the test function, the message was redeemed from the receiveAvail instead of receiveERC20 and was still successful, causing a large imbalance of value coming in and value being redeemed

Impact

Loss of funds for the protocol

Code Snippet

https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L383

https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L212

Tool used

Manual Review

Recommendation

Add check to ensure assetId in message matches avail assetId before minting avail to receiver

sherlock-admin commented 8 months ago

2 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Invallid. Admin mistake

takarez commented:

invalid because {invalid: intended behavior}