Submitted by carrotsmuggler, also found by KIntern_NA
The function removeCollateral is used to trigger a removal of collateral on a different chain. The function takes the parameter from, which is the account whose collateral will be sold. It also takes the parameter to where these collateral tokens will be transferred to.
The main issue is that anyone can call this function with any address passed to the from parameter. There is no allowance check on the chain, allowing this operation. Lets walk through the steps to see how this is executed. Lets assume both from and to are the victims address for reasons explained at the end.
BaseTOFT.sol:removeCollateral is called by the attacker with a from and to address of the victim. This function calls the removeCollateral function in the market module.
BaseTOFTMarketModule.sol:removeCollateral is called. This function packs some data and sends it forward to the lz endpoint. Point to note, is that no approval check is done for the msg.sender of this whole setup yet.
After the message is sent, the lzendpoint on the receiving chain will call the TOFT contract again. Now, the msg.sender is not the attacker, but is instead the lzendpoint! The endpoint call gets delegated to the remove function in the Market module.
Here we see the unpacking. note that the third unpacked value is put in the to field. This is an address determined by the attacker and passed through the layerzero endpoints. The contract then calls a market contracts removeCollateral function.
Thus it is evident from this call that the Market contract being called has no idea that the original sender was the attacker. Instead, for the Market contract, the msg.sender is the current TOFT contract. If users want to use the cross chain operations, they have to give allowance to the TOFT contract address. Thus we can assume that the victim has already given allowance to this address. Thus the market thinks the msg.sender is the TOFT contract, who is allowed, and thus executes the operations.
Thus we have demonstrated that the attacker is able to call a function on the victims market position without being given any allowance by setting the victims address in the to field. While it is a suspected bug that the removeCollateral removes collateral from the to fields account and not the from field, since both these parameters are determined by the attacker, the bug exists either way. Thus this is a high severity issue since the victims collateral is withdrawn, dropping their health factor.
A POC isnt provided since the test suite does not have a test for the removeCollateral function. However the function retrieveFromStrategy suffers from the same issue and has been addressed in a different report. The test for that function can be used to demonstrate this issue.
Two lines from the test in test/TapiocaOFT.test.ts is changed to show this issue. Below is the full test for reference. The changed bits are marked with arrows.
The only relevant change is that the function retrieveFromStrategy is called from another address. The test passes, showing that an attacker, in tthis case randomUser can influence the operations of the victim, the signer.
Add an allowance check for the msg.sender in the removeCollateral function.
0xRektora (Tapioca) confirmed
LSDan (Judge) commented:
