TOFT can be forcefully unwrapped resulting in long-term DoS
Summary
TOFTGenericModule::receiveWithParamsReceiver function can be abused to wipe out anyone's allowance of the TOFT token.
At the same time, it unwraps the TOFT. As a result, the allowance mechanism is broken and there is no point in wrapping your tokens in an OFT as a griever can immediately unwrap them.
This enters the _internalTransferWithAllowance function where _owner = Alice, srcChainSender = Bob, and amount = 100.
_spendAllowance is called, and it decrements the allowance of Bob by 100, i.e. _allowances[Alice][Bob] = 0.
_transfer transfers the OFTs to the address(this).
tOFT.unwrap(address(this), msg_.amount); is called, and the OFTs are unwrapped to the address(this).
The erc20 safeTransfer function is called, and the erc20 is transferred to Alice.
A griever has forcefully wiped out the allowance of Bob and unwrapped Alice's OFT.
This makes a critical vulnerability as it means there is no point in wrapping your tokens in an OFT as a griever can immediately unwrap them. Also, the allowance mechanism is broken as it can be wiped out by anyone.
Any system that accepts OFTs as collateral or otherwise won't work as no one can wrap their tokens in an OFT as they can be unwrapped immediately by anyone.
As giving allowances to external systems to use your TOFT tokens is a typical flow the likelihood of this occurring all the time is high.
Impact
The impact of this vulnerability is critical. It allows anyone to wipe out the allowance of any user and unwrap their OFT tokens. This makes the allowance mechanism broken and as the TOFT token is a wrapper token, it makes it useless as a griever can immediately unwrap them.
The whole receiveWithParamsReceiver function should be reviewed and refactored. It is not clear what the expected behavior is.
There should be a different flow if used as part of the cross-chain flow as opposed to being callable on the same chain.
If used on the same chain, the msg.sender should only be able to transfer the OFT tokens if the owner has given him sufficient allowance. Check the typical transferFrom implementation:
function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) {
address spender = msg.sender;
_spendAllowance(from, spender, amount);
_transfer(from, to, amount);
return true;
}
If used in the cross-chain context, you can rely on the srcChainSender as the owner of the TOFT tokens, and he can unwrap them to an arbitrary receiver address.
GiuseppeDeLaZara
high
TOFT can be forcefully unwrapped resulting in long-term DoS
Summary
TOFTGenericModule::receiveWithParamsReceiver
function can be abused to wipe out anyone's allowance of the TOFT token. At the same time, it unwraps the TOFT. As a result, the allowance mechanism is broken and there is no point in wrapping your tokens in an OFT as a griever can immediately unwrap them.Vulnerability Detail
TOFTGenericModule::receiveWithParamsReceiver
function can be used to wipe out the allowance of the TOFT token by anyone and unwrap TOFT tokens.Let's imagine the following scenario:
_allowances
mapping, i.e._allowances[Alice][Bob] = 100
;TOFTGenericModule::receiveWithParamsReceiver
is callable by anyone through theTOFT::executeModule
function._internalTransferWithAllowance
function where_owner = Alice
,srcChainSender = Bob
, andamount = 100
._spendAllowance
is called, and it decrements the allowance of Bob by 100, i.e._allowances[Alice][Bob] = 0
._transfer
transfers the OFTs to theaddress(this)
.tOFT.unwrap(address(this), msg_.amount);
is called, and the OFTs are unwrapped to theaddress(this)
.safeTransfer
function is called, and theerc20
is transferred toAlice
.A griever has forcefully wiped out the allowance of Bob and unwrapped Alice's OFT.
This makes a critical vulnerability as it means there is no point in wrapping your tokens in an OFT as a griever can immediately unwrap them. Also, the allowance mechanism is broken as it can be wiped out by anyone.
Any system that accepts OFTs as collateral or otherwise won't work as no one can wrap their tokens in an OFT as they can be unwrapped immediately by anyone.
As giving allowances to external systems to use your TOFT tokens is a typical flow the likelihood of this occurring all the time is high.
Impact
The impact of this vulnerability is critical. It allows anyone to wipe out the allowance of any user and unwrap their OFT tokens. This makes the allowance mechanism broken and as the TOFT token is a wrapper token, it makes it useless as a griever can immediately unwrap them.
Code Snippet
Tool used
Manual Review
Recommendation
The whole
receiveWithParamsReceiver
function should be reviewed and refactored. It is not clear what the expected behavior is.There should be a different flow if used as part of the cross-chain flow as opposed to being callable on the same chain.
If used on the same chain, the
msg.sender
should only be able to transfer the OFT tokens if the owner has given him sufficient allowance. Check the typicaltransferFrom
implementation:If used in the cross-chain context, you can rely on the
srcChainSender
as the owner of the TOFT tokens, and he can unwrap them to an arbitrary receiver address.Duplicate of #134