Blocklist and SanctionList can be escaped by queueing bridge transaction

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/DestinationBridge.sol#L90 https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/bridge/SourceBridge.sol#L91

Vulnerability details

Impact

Blocklist and SanctionList can be gamed by queueing bridge transaction

Proof of Concept

To comply with law, the USDY and rUSDY token implement blocklist and SanctionList

when a address is added to blocklist or scantionlist, the address should not be able to receive token or transfer token

However, this blocklist / sanction list can be gamed by submiting a bridge transaction on source chain

In the current implementation, when use the source bridge, first the source token is burnt

then the user pay the gas fee and a cross-chain function call is made by calling AXELAR_GATEWAY.callContract

the relevant logic is here

there are two part of fee

1. the gas fee paid to execute the transction the burnAndCallAxelar
2. the network base fee and execution fee that needs to send GAS_RECEIVER to let relayer on axelar side successly executes the transaction

if a user detects that he will be blocklisted or sanctioned,

he can submit a burnAndCallAxelar by paying enough gas to executes the transaction to burn his token on source chain, but he can intentionally underpays the execution fee to GAS_RECEIVER to not let the relay transaction executes immediately

then the transaction triggered on destion bridge and the token is burnt on source chain but minted on dest chain

there is a few technical details, when executes on dest chain, the payload is extracted in this line of code

  function _execute(
    string calldata srcChain,
    string calldata srcAddr,
    bytes calldata payload
  ) internal override whenNotPaused {
    (bytes32 version, address srcSender, uint256 amt, uint256 nonce) = abi
      .decode(payload, (bytes32, address, uint256, uint256));

the srcSender refers to the msg.sender in source chain

and the token will be minted to the same srcSender on dest chain

     TOKEN.mint(txn.sender, txn.amount);

there are two cases, if the msg.sender in source chain is a EOA address or a smart contract address

In conclusion:

1. if an msg.sender is a EOA account and admin blocklist the address in source chain but not on dest chain, the Blocklist and SanctionList can be escaped if there are pending bridge transaction

2. if an msg.sender is a smart contract account and admin cannot block same smart contract on source chain and dest chain because the same smart contract may have different owner in dest chain, the Blocklist and SanctionList can be escaped if there are pending bridge transaction

suppose I hold a EOA address: 0x14dC79964da2C08b23698B3D3cc7Ca32193d9955

I bridge 1000 USDY from ethereum to optimism

the token is burnt from address 0x14dC79964da2C08b23698B3D3cc7Ca32193d9955 in ethereum and the same amount receved on optimism in happy flow

but my account is blocklisted in ethereum, that does not mean my account is blocklisted in optimism as long as there are pending cross-chain bridge transaction via axelar

if the admin blocklist the same EOA address across all network (including both ethereum and optimism, this is not a issue, I cannot receive the minted token on optimism)

so one may argue as long as the admin blocklist the same address across all network, blocklist works

but but but

suppose I hold a smart contract address:

I bridge 1000 USDY from ethereum to optimism

the token is burnt from smart contract address on ethreum, and the same amount received for the same smart contract address

in this case, the admin cannot really blocklist both smart contract on both ethereum and optimism

because the same smart contract on different network may have different owner and the owner of the optimism smart contract does not misbehave so there is no reason for admin to blocklist the same smart contract address in optimism

so this mean the admin can blocklist the smart contract on ethereum, but the same smart contract may be owned by different person on optimism

so admin blocklist the same address across all network does not really work

Tools Used

Manual Review

Recommended Mitigation Steps

there are a few solution, one of the solution is not let smart contract call bridge transaction or only let whitelisted smart contract call bridge transaction

then a user cannot use smart contract to queueing pending transaction to escape blocklist and sanction

Assessed type

Timing

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as high quality report

[c4-sponsor]

tom2o17 (sponsor) disputed

[kirk-baird]

Not valid as the user will have isAllowed() return false when attempting to bridged on destination chain. The net result is the bridged transaction cannot be executed.

_beforeTokenTransfer() is triggered on the destination chain when minting. It will cause reject sanctioned users. See here

[c4-judge]

kirk-baird marked the issue as unsatisfactory: Invalid

[JeffCX]

Emmm, I highly respect judge's expertise, but I think I really want to elaborate:

Not valid as the user will have isAllowed() return false when attempting to bridged on destination chain. The net result is the bridged transaction cannot be executed.

As the report shown

the malicious actor does not have to frontrun the blacklist transaction,

they can send the cross-chain transaction by burning their token in source chain and intentionally underpay the gas long before they get blacklisted

so the cross-chain message is never relayed, but it is queud

then after the address is blacklisted in source chain

the user can increase the gas so the cross-chain transaction is relayed and the blacklisted user is able to receive the asset in dest chain

https://docs.axelar.dev/dev/general-message-passing/gas-services/increase-gas#increase-gas

again, as the report highlight, the sender address that is blacklisted can be a smart contract

and the owner of the address is source chain and dest chain can be different person

so adding both smart contract address in source chain and dest chain to blocklist basically means the protocol sanction the wrong target

_beforeTokenTransfer() is triggered on the destination chain when minting. It will cause reject sanctioned users.

as for the allowlist, if the target is the smart contract address

it is always return True in the allowlist because the allowlist check always return True if the target is a smart contract when the minting transaction is landed in dest chain

emm so I politely think the severity is medium because the sancation mechanism can by gamed if there are pending corss-chain transaction, andI I think only https://github.com/code-423n4/2023-09-ondo-findings/issues/396 is the duplicate because other submission does not really mention the cross-chain part

[kirk-baird]

@JeffCX Thanks for the feedback you're correct that smart contracts are treated differently in the allow list and always pass. Therefore my initial response that _beforeTokenTransfer() handles the isAllowed() is incorrect. However, I believe we'll still have issues for isBlocked() and isSanctioned() as they do not have the smart contract logic.

In terms of validity of this issue we've established that the Allowlist can be bypassed due to the is contract check. However, I don't see how we are able to pass the isBlocked() or isSanctioned() checks here.

For example isBlocked() does not differ in it's implementation based on whether the address is a smart contract or not.

Agreed that this issue and #396 are duplicates not related to the others. The other issues are related to front-running as opposed to using smart contracts over the bridge are a different set of issue and will be downgraded to QA.

[JeffCX]

Thanks for the response

In terms of validity of this issue we've established that the Allowlist can be bypassed due to the is contract check. However, I don't see how we are able to pass the isBlocked() or isSanctioned() checks here.

For example isBlocked() does not differ in it's implementation based on whether the address is a smart contract or not.

this is the issue, as highlight in the original submission

oh well

assume the blacklisted address in source chain is a smart contract and have pending transaction to mint token in dest chain

because the same address in different chain may have different owner or logic, protocol cannot blacklist the same address in dest chain to not let mint transaction land

how to prove?

ok let us revisit the hack

https://rekt.news/wintermute-rekt/

optimism want to send the 20M OP token to the wintermute multisig address

wintermute control the multisig address in ethereum mainnet

but no one control the same multisig address in OP network

and optimism team send the the 20M OP token to the address in optimism

the hacker, who is able to extract the genosis safe factory nonce, the deploy the address before the wintermute team to control the 20M OP token

ok assume the lost token is 20M USDY ondo token

and the hacker has pending cross-chain transaction, the token is already burned in the source chain (optimism)

does it make sense to blocklist the same address in ethereum? no,

the same address owned by hacker in optimism can be rightfully blocked

but the same multisig wallet address in ethereum is owned by wintermute (other owner)

if the protocol want to prevent the mint transaction landing, they block the same multisig wallet in ethereum, they wrongly block all in-bounding transfer / outbonding transfer of a different owner in ethereum

ok, let us use a more extreme case to domonstrate how hacker can get the money out by using cross-chain transfer

https://twitter.com/sstrenev/status/1702345227808219593

in this twitter, the tweet post highlight a low-efforts MEV bot, the bot backrun the lido ETH positive rebase by calling the uniswap V2 skim function to extract the additional stETH

skim function is a way to get the token out

https://twitter.com/sstrenev/status/1702345231922860391

To those that are not familiar with what skim() does - it is a function that anyone can call on a pair contract.

It checks the differences between the internal and actual balances of the tokens and transfers the difference to an address specified by the caller.

this is the Uniswap V2 stETH / WETH pool

https://etherscan.io/address/0x4028daac072e492d34a3afdbef0ba7e35d8b55c4#writeContract

assume the token pair address is USDY / WETH address in Uniswap V2 in mainnet

and the same smart contract address in polygon get blocklisted but has queuing cross-chain transaction from polygon to mainnet and the USDY will be mint to token pair is USDY / WETH address in Uniswap V2

the protocol cannot blacklist the the token pair is USDY / WETH address in Uniswap V2 because it will block the trading of USDY

and when the asset is minted to USDY / WETH address in Uniswap V2, the blacklisted user can use another account to call skim to get the token out

emm the comment is long, but my main point is

For example isBlocked() does not differ in it's implementation based on whether the address is a smart contract or not.

the protocol is at the high risk of blocklisting wrong address in the dest chain to block the cross-chain token minting by a blacklisted address in source chain

but if protocol does not blocklist the same smart contract address in dest chain,

Blocklist and SanctionList can be escaped by queueing bridge transaction and the mint transaction will land on dest chain, the blacklisted user may or may not get the token out in dest chain but that violate the invariant: blacklisted / blocklisted user cannot transfer the out

[stalinMacias]

I was interested in the discussion about this report and I thought I might add something, but looks like what I was thinking to add, the good @tom2o17 has already written it in a different report, see here.

So based on what @tom2o17 mentioned, I think the key is to understand that the fact that the call to bridge tokens in the destination chain is processed by Axelar, that doesn't automatically mean that those tokens will be minted in reality, as tom mentioned in the other report, depending on the number of tokens being minted will be the number of approvers required to authorize the minting of those tokens, one could argue that the malicious user could send many bridge operation with a low amount of tokens each, but I think it's wrong to assume that the lowest tier of the approversThreshold will be only 1 approver, i.e, To mint from 0-100 USDY that is required only 1 approver, this is not a fact since the protocol can set any number of approvers, it could be 2, and this means that to mint any amount of USDY in the destination chain it will be require, at least, 1 approver from the ONDO Team....

I think that the approval mechanism correctly protects the protocol from users trying to abuse what's described in this report.

I just wanted to share my thoughts about this finding because I also thought that this could be a vulnerability but when I dug more in the code I realized that it was a false-positive and that's why I didn't submit it, but I may be missing something, please, feel free to prove me wrong about my conclusions 💯

[JeffCX]

I think that the approval mechanism correctly protects the protocol from users trying to abuse what's described in this report.

thanks for the input, based on the comments above, looks like the approver can just not approve the message to not let the abuse described in this report work

not a issue I guess

looks like the approver needs to check whether the address sender in source chain is blocklisted before approve the message, but how does the approver do is out of scope

[kirk-baird]

This is an interesting edge case that can be prevent by approvers off-chain or by applying the blocklist / sanction list to all chains at the same time.

Since the resolution for this issue is to improve the off-chain operations I'll adjust this issue to be QA.

[c4-judge]

kirk-baird removed the grade

[c4-judge]

kirk-baird changed the severity to QA (Quality Assurance)

[c4-judge]

kirk-baird marked the issue as grade-b