0xKartikgiri00 - In `BridgeRelay` contract the user MATIC balance can lost permanently.

[sherlock-admin4]

0xKartikgiri00

high

In BridgeRelay contract the user MATIC balance can lost permanently.

Summary

The contract BridgeRelay allows for the transfer of MATIC tokens to a destination account. However, if multiple accounts transfer MATIC tokens to this contract, all the MATIC balance will be sent to a single user address.

Vulnerability Detail

In the BridgeRelay::erc20Rescue function, which allows the contract owner to return MATIC tokens from the contract to a specified destination. Since there is no logic implemented to differentiate between different users who might have deposited MATIC tokens directly to BridgeRelay contract, all the MATIC tokens held by the contract will be sent to the single user address specified by the owner, regardless to the original depositors.

Impact

The impact is that if multiple users deposit MATIC tokens to the contract, intending to bridge them to another network or perform some other operation, the entire MATIC balance will be redirected to a single address determined by the owner. This could result in loss of funds for the depositors and disrupt the intended functionality of the contract.

Proof Of Concept

The below test case shows how an attacker can transfer matic token to BridgeRelay just right after the genuine user transaction and can get the whole amount of matic token available in the contract.

FOUNDRY TEST CASE:-

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.24;

import {Test, console} from "forge-std/Test.sol";
import {MockBridgeRelay} from "../src/bridge/test/MockBridgeRelay.sol";
import {TestPOSBridge} from "../src/bridge/test/TestPOSBridge.sol";
import {TestPredicate} from "../src/bridge/test/TestPredicate.sol";
import {DeployBridge} from "../script/DeployBridge.s.sol";
import {TestToken} from "../src/test/TestToken.sol";

contract BridgeRelayTest is Test {
    DeployBridge public _deployer;
    MockBridgeRelay public _BridgeRelay;
    TestPredicate public _Predicate;
    TestPOSBridge public _POSBridge;
    TestToken public _weth;
    TestToken public _matic;

    address public _Owner;
    address public User1 = makeAddr("user1");
    address public User2 = makeAddr("user2");

    uint256 public startingBal = 5 ether;

    function setUp() public {
        _deployer = new DeployBridge();
        (_BridgeRelay, _Predicate, _POSBridge, _Owner, _weth, _matic) = _deployer.run();
        _matic.mintTo(User1, startingBal);
        _matic.mintTo(User2, startingBal);
        _weth.mintTo(User1, startingBal);
        _weth.mintTo(User2, startingBal);
    }

    function test_ERC20Rescue() public {
        vm.prank(User1);
        _matic.transfer(address(_BridgeRelay), 2 ether);
        console.log("The matic balance of BridgeRelay after user1 deposit:", _matic.balanceOf(address(_BridgeRelay)));
        console.log("The matic balance of User1 after transfer:", _matic.balanceOf(User1));
        vm.prank(User2);
        _matic.transfer(address(_BridgeRelay), 2 ether);
        console.log("The matic balance of BridgeRelay after user2 deposit:", _matic.balanceOf(address(_BridgeRelay)));
        console.log("The matic balance of User2 after transfer:", _matic.balanceOf(User2));
        vm.prank(_Owner);
        _BridgeRelay.erc20Rescue(User2);
        console.log(
            "The matic balance  of BridgeRelay after rescue call by owner:", _matic.balanceOf(address(_BridgeRelay))
        );
        console.log("The matic balance of User1:", _matic.balanceOf(User1));
        console.log("The matic balance of User2:", _matic.balanceOf(User2));
        //The user2  matic balnce will be 7 after erc20rescue.
        assertEq(7 ether, _matic.balanceOf(User2));
        //The user1  matic balnce will be 3 after erc20rescue.
        assertEq(3 ether, _matic.balanceOf(User1));
    }

}

FOUNDRY DEPLOYEMENT SCRIPT:-

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.24;

import {Script} from "forge-std/Script.sol";
import {console} from "forge-std/Test.sol";
import {MockBridgeRelay} from "../src/bridge/test/MockBridgeRelay.sol";
import {TestPOSBridge} from "../src/bridge/test/TestPOSBridge.sol";
import {TestPredicate} from "../src/bridge/test/TestPredicate.sol";
import {TestToken} from "../src/test/TestToken.sol";

contract DeployBridge is Script {
    address public Owner = makeAddr("owner");

    function run() external returns (MockBridgeRelay, TestPredicate, TestPOSBridge, address, TestToken, TestToken) {
        vm.startBroadcast();
        TestToken weth = new TestToken("weth", "weth", 18, address(Owner), 1000000000000000000000);
        TestToken matic = new TestToken("matic", "matic", 18, address(Owner), 1000000000000000000000);

        TestPredicate Predicate = new TestPredicate();
        TestPOSBridge PosBridge = new TestPOSBridge(Predicate);

        MockBridgeRelay BridgeRelay = new MockBridgeRelay(weth, matic, PosBridge, address(Predicate), Owner);
        vm.stopBroadcast();

        console.log("BridgRelay contract address: ", address(BridgeRelay));

        return (BridgeRelay, Predicate, PosBridge, Owner, weth, matic);
    }
}

Output:-

[PASS] test_ERC20Rescue() (gas: 67887)
Logs:
  BridgRelay contract address:  0x50EEf481cae4250d252Ae577A09bF514f224C6C4
  The matic balance of BridgeRelay after user1 deposit: 2000000000000000000
  The matic balance of User1 after transfer: 3000000000000000000
  The matic balance of BridgeRelay after user2 deposit: 4000000000000000000
  The matic balance of User2 after transfer: 3000000000000000000
  The matic balance  of BridgeRelay after rescue call by owner: 0
  The matic balance of User1: 3000000000000000000
  The matic balance of User2: 7000000000000000000

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.56ms

Code Snippet

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/bridge/BridgeRelay.sol#L88 https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/bridge/BridgeRelay.sol#L95

Tool used

Manual Review, Foundry

Recommendation

The recommended mitigation can be is to add new param amount in the erc20Rescue function so that only specified amount of the matic token which transfer by user in BridgeRelay contract is return to the user not the whole matic amount of contract.

[sherlock-admin4]

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

WangAudit commented:

the only public function (bridgeTransfer) doesn't allow matic and reverts; if matic is sent via transfer; then it's user mistake -> invalid under Sherlock's rules

[kartik-giri]

Even though, it's a valid issue.

[kartik-giri]

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

WangAudit commented:

the only public function (bridgeTransfer) doesn't allow matic and reverts; if matic is sent via transfer; then it's user mistake -> invalid under Sherlock's rules

Than it is ok if user lost funds?