TempleGold

TempleDAO
Foundry
25,000 USDC
View results
Submission Details
Severity: high
Valid

Contract Accounts are at risk of losing TGLD when bridging if they don't have access to same address at destination chain.

Relevant GitHub links

https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGold.sol#L303-#L306

Summary

Contract accounts (multisig wallets / DApps) are at risk of losing funds in the process of bridging.

Vulnerability Details

Tokens might get minted to address controlled by a user who may decide not to refund them.
Attacker might be able to deploy multisig controlled by them on destination chain.
See [Wintermute](https://rekt.news/wintermute-rekt/)

Impact

TGLD tokens would be minted to an address not owned by the user (contract account), leading to loss of funds.

Tools Used

Manual Review

Recommendations

TGLD contract needs to maintain another list which allows the users (contract accounts) to accept that they have the control of same address on destination chain. This list needs to be checked before any cross-chain transfer if the user has enabled/accepted cross-chain transfer for their contract account.

Add following functionality:

mapping(address user => mapping (uint32 chainId => bool allowed)) public bridgeAllowList;
function acceptBridgingToChain(uint32 chainId) external {
bridgeAllowList[msg.sender][chainId] = true;
// Can also make a call to destination chain using layerzero
// To mark that this address is controlled on current chain as well.
}

This ensures that user is aware of bridging risks and accepts that they are the rightful owner of contract account on destination chain.

Now we need to add a revert statement in send method for veryifying user's acceptance.

function send(
SendParam calldata _sendParam,
MessagingFee calldata _fee,
address _refundAddress
) external payable virtual override(IOFT, OFTCore) returns (MessagingReceipt memory msgReceipt, OFTReceipt memory oftReceipt) {
if (_sendParam.composeMsg.length > 0) { revert CannotCompose(); }
/// cast bytes32 to address
address _to = _sendParam.to.bytes32ToAddress();
/// @dev user can cross-chain transfer to self
+ if (msg.sender != tx.origin)
+ if (!bridgeAllowList[msg.sender][_sendParam.dstEid])
+ revert();
if (msg.sender != _to) { revert ITempleGold.NonTransferrable(msg.sender, _to); }
// @dev Applies the token transfers regarding this send() operation.
// - amountSentLD is the amount in local decimals that was ACTUALLY sent/debited from the sender.
// - amountReceivedLD is the amount in local decimals that will be received/credited to the recipient on the remote OFT instance.
(uint256 amountSentLD, uint256 amountReceivedLD) = _debit(
msg.sender,
_sendParam.amountLD,
_sendParam.minAmountLD,
_sendParam.dstEid
);
// @dev Builds the options and OFT message to quote in the endpoint.
(bytes memory message, bytes memory options) = _buildMsgAndOptions(_sendParam, amountReceivedLD);
// @dev Sends the message to the LayerZero endpoint and returns the LayerZero msg receipt.
msgReceipt = _lzSend(_sendParam.dstEid, message, options, _fee, _refundAddress);
// @dev Formulate the OFT receipt.
oftReceipt = OFTReceipt(amountSentLD, amountReceivedLD);
emit OFTSent(msgReceipt.guid, _sendParam.dstEid, msg.sender, amountSentLD, amountReceivedLD);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Account abstraction, Multisig, Any other contract based solution that doesn't share the same address across chains will lose it's TGLD in teleport.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.