TempleGold

TempleDAO
Foundry
25,000 USDC
View results
Submission Details
Severity: low
Invalid

Teleporting is not done in the most effective way and could lead to loss of funds for to some users

Summary

Teleporting is not done in the most effective way and could lead to loss of funds for to some users or even it being unavailable for them.

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-07-templegold/blob/a47602635bf55221404fc9ceb96a8bb6c0db36a2/protocol/contracts/templegold/TempleTeleporter.sol#L43-L58

function teleport(
uint32 dstEid,
address to,
uint256 amount,
bytes calldata options
) external payable override returns(MessagingReceipt memory receipt) {
if (amount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
if (to == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
// Encodes the message before invoking _lzSend.
bytes memory _payload = abi.encodePacked(to.addressToBytes32(), amount);
// debit
temple.burnFrom(msg.sender, amount);
emit TempleTeleported(dstEid, msg.sender, to, amount);
receipt = _lzSend(dstEid, _payload, options, MessagingFee(msg.value, 0), payable(msg.sender));
}

This function is used to teleport temple tokens cross chain, note that this function uses the Layer Zero _lzsend() to do this.

Issue however is that this function hardcodes who the address where the funds would be refunded to, i.e payable(msg.sender), now naturally we would expect the normal functionality to expect users to attach the msg.value a bit over the gas that is expected so as to ensure the transaction is successful, i.e there would always be a gas to refund, otherwise we are expecting the users to guess correctly right down to the wei value how much gas they would need for the transaction which would be impossible.

Now the issue with the current implementation is that hardcoding the payable(msg.sender) as the refund address would not work as expected and allow the cross-chaining process to be as smooth as usual.

This is because if this address (payable(msg.sender)) is a contract that can't receive ether value or have no way to withdraw this ether, then the refunded ether are effectively lost, or the cross chaining would not work due to a revert if the address can't receive ether.

Impact

As hinted under Vulnerability Details, in the case where an address can not receive ether, then they would not be able to teleport their temple tokens, cause we can't expect them to get the exact amount of gas needed right down to the wei value, and this is also the intended functionality which is why in the first place there is a refunding to happen via the _lzsend() call.

Now in the case where there is a need to refund and the payable(msg.sender) is a contract address that has no functionality to retrieve the eth then the user's tokens are effectively lost.

Tools Used

Manual review

Recommendations

Consider introducing a functionality that would allow for users to specify their own refund address.

function teleport(
uint32 dstEid,
address to,
uint256 amount,
bytes calldata options
+ address refundAddress,
) external payable override returns(MessagingReceipt memory receipt) {
if (amount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
if (to == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
// Encodes the message before invoking _lzSend.
bytes memory _payload = abi.encodePacked(to.addressToBytes32(), amount);
// debit
temple.burnFrom(msg.sender, amount);
emit TempleTeleported(dstEid, msg.sender, to, amount);
- receipt = _lzSend(dstEid, _payload, options, MessagingFee(msg.value, 0), payable(msg.sender));
+ receipt = _lzSend(dstEid, _payload, options, MessagingFee(msg.value, 0), refundAddress);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Hardcoding the `refundAddress` to `payable(msg.sender)` might lead to stuck fees in case of contracts that don't expect them.

Appeal created

bauchibred Submitter
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Hardcoding the `refundAddress` to `payable(msg.sender)` might lead to stuck fees in case of contracts that don't expect them.

Support

FAQs

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