something wrong at 7-bossbridge

[jupiter-Pulin]

What I don't understand is how it can fabricate signature information. Specifically, in the source code, if you want to call the sendToL1 function, it requires VRS, and this VRS must be signed by the signer themselves using their private key. The test code here directly simulates this, but that's impossible! Because in reality, the signer would never expose their private key. Consequently, the data: approveTo that I want to call in my abiencode cannot be achieved.

// Under the assumption that the bridge operator doesn't validate bytes being signed
        bytes memory message = abi.encode(
            address(vault), // target
            0, // value
            abi.encodeCall(L1Vault.approveTo, (address(attacker), type(uint256).max)) // data
        );
        (uint8 v, bytes32 r, bytes32 s) = _signMessage(message, operator.key);

[EngrPips]

What is the vulnerability about?

[jupiter-Pulin]

here it is

L1BossBridge::sendToL1 allowing arbitrary calls enables users to call L1Vault::approveTo and give themselves infinite allowance of vault funds
The L1BossBridge contract includes the sendToL1 function that, if called with a valid signature by an operator, can execute arbitrary low-level calls to any given target.
Because there's no restrictions neither on the target nor the calldata, this call could be used by an attacker to execute sensitive contracts of the bridge. For example, the L1Vault contract.

The L1BossBridge contract owns the L1Vault contract. Therefore, an attacker could submit a call that targets the vault and executes is approveTo function, passing an attacker-controlled address to increase its allowance. This would then allow the attacker to completely drain the vault.

It's worth noting that this attack's likelihood depends on the level of sophistication of the off-chain validations implemented by the operators that approve and sign withdrawals.
However, we're rating it as a High severity issue because, according to the available documentation, the only validation made by off-chain services is that "the account submitting the withdrawal has first originated a successful deposit in the L1 part of the bridge".
As the next PoC shows, such validation is not enough to prevent the attack.

    function sendToL1(uint8 v, bytes32 r, bytes32 s, bytes memory message) public nonReentrant whenNotPaused {
        address signer = ECDSA.recover(MessageHashUtils.toEthSignedMessageHash(keccak256(message)), v, r, s);

        if (!signers[signer]) {
            revert L1BossBridge__Unauthorized();
        }

        (address target, uint256 value, bytes memory data) = abi.decode(message, (address, uint256, bytes));

        (bool success,) = target.call{ value: value }(data);
        if (!success) {
            revert L1BossBridge__CallFailed();
        }
    }

[EngrPips]

If a user can approve themself of all the balance of the L1Vault contract, then they can drain it as you have mentioned. As you mentioned earlier, the user does not need to fabricate a signature. The user needs to get a real, signed message that accounts for what they have deposited into the protocol and then continue using that message to drain the protocol.

The reason why that will work is because the sendToL1 function below does not check if the message has already been redeemed, but only checks if it was signed by a signer, which is indeed true.

function sendToL1(uint8 v, bytes32 r, bytes32 s, bytes memory message) public nonReentrant whenNotPaused {
        address signer = ECDSA.recover(MessageHashUtils.toEthSignedMessageHash(keccak256(message)), v, r, s);

        if (!signers[signer]) {
            revert L1BossBridge__Unauthorized();
        }

        (address target, uint256 value, bytes memory data) = abi.decode(message, (address, uint256, bytes));

        (bool success,) = target.call{ value: value }(data);
        if (!success) {
            revert L1BossBridge__CallFailed();
        }
    }

Please ask questions if anything is still confusing.

[jupiter-Pulin]

the condition for this test code to hold is that the signer must sign this approveTo ABI transaction, but isn't this very unlikely in reality? Because the signer would check if the user is calling the withdrew function or check if the ToL1 ABI information of the message is reasonable.

By the way: are custom bridges relatively rare now, with CCIP being the more common form?

[EngrPips]

Well, I think you may have a point about the check, but then the possibility is there, I mean, we saw the Bybit Exchange got hacked, and that hack required 3 signers from Bybit, and they all signed the malicious transaction.

About what bridge is common nowadays, I honestly don't know, as I don't use bridges that much, and I haven't done any personal research about bridges.