Anchor contest - broccoli's results

Lines of code

https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L173 https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L251 https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L256

Vulnerability details

Impact

The CrossAnchorBridge contract accepts any ERC20 token and transfers them to the wormhole bridge. There were allowlist checks on the tokens before, but they were commented out in this version for the audit. If a user transfers, for example, non-supported collateral to the bridge contract, the bridge on the Ethereum side will accept it, but the lending market on the Terra side will reject it. As a result, the transferred token could be lost in the bridge contract and is unrecoverable.

Proof of Concept

The following allowlist checks in the CrossAnchorBridge are commented out: CrossAnchorBridge.sol#L173 CrossAnchorBridge.sol#L251 CrossAnchorBridge.sol#L256

However, there are allowlist restrictions on the tokens on the money market contracts, for example: overseer/src/collateral.rs#L37 overseer/src/collateral.rs#L99 overseer/src/state.rs#L77-L89

Recommended Mitigation Steps

Uncomment the allowlist checks in the referenced lines of code. Besides, would suggest adding functionality to add or remove tokens from the allowlist dynamically.

Lines of code

https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/money-market-contracts/contracts/custody_beth/src/distribution.rs#L110-L115 https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/money-market-contracts/contracts/custody_bluna/src/distribution.rs#L109-L114

Vulnerability details

Impact

The swap_to_stable_denom function in the custody contracts swaps all other native tokens into a specific one. The function creates swap messages for all the other native tokens and adds them as sub-messages, and handles the reply only when the last sub-message succeeds. Upon receiving the reply, the contract sends the swapped tokens (i.e., rewards) to the overseer contract.

In cases where the last sub-message fails, the custody contract will not receive a reply, and therefore the rewards are left in the contract. The rewards are locked in the contract until someone triggers swap_to_stable_denom again, and the last swap succeeds. However, if the last swap consistently fails in some period for any reason, the total rewards will be locked in the contract during that period. As a result, users cannot get the rewards they are supposed to receive in that period.

Proof of Concept

Referenced code: custody_beth/src/distribution.rs#L110-L115 custody_bluna/src/distribution.rs#L109-L114

Recommended Mitigation Steps

Consider handling the reply on either success or failure, i.e., using ReplyOn::Always, to avoid the failure of the swap to cause tokens to be locked.

Lines of code

https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/cross-chain-contracts/ethereum/CrossAnchorBridge.sol#L183-L201

Vulnerability details

Impact

The handleToken function of CrossAnchorBridge does not correctly handle the received amount of fee-on-transfer tokens. For fee-on-transfer tokens (e.g., USDT), the received amount could be less than the requested amount because of the deducted fee. As a result, the subsequent call to the wormhole token bridge will fail because the anchor bridge does not own that many tokens, causing the whole transaction to revert.

Proof of Concept

Referenced code:

ethereum/CrossAnchorBridge.sol#L183-L201

Recommended Mitigation Steps

If the anchor bridge is supposed to support fee-on-transfer tokens, get the real received amount by calculating the balance difference before and after the transfer.