Background
On October 8, 2020, the decentralized wallet imToken tweeted that users reported that 310,000 DAI had been stolen, which was related to the vulnerability of DeFi Saver Exchange. DeFi Saver responded that the stolen funds are still safe and are contacting the victim users. Up to now, all funds have been returned to the victimized users. As early as June this year, DeFi Saver stated that the team discovered a vulnerability in its own trading platform in the DeFi Saver application series. The 310,000 DAI theft was also related to the previous SaverExchange contract vulnerability. After receiving the intelligence, the SlowMist security team conducted a specific analysis of the 310,000 DAI theft.
Attack process analysis
View this attack transaction:
https://etherscan.io/tx/0xcd9dad40b409897d05fa0e60ed4e58eb99876febf94bc97679b7f45837ea86b7
It can be seen that the stolen user 0xc0 directly transferred 310,000 DAI to the attack contract 0x5b.
We can use OKO browser to view specific transaction details:
https://oko.palkeo.com/0xcd9dad40b409897d05fa0e60ed4e58eb99876febf94bc97679b7f45837ea86b7/
It can be seen that the attacker passes in _exchangeAddress by calling the swapTokenToToken function, _src, _dest are the DAI contract addresses, selects _exchangeType as 4, and passes in a custom _callData. It can be guessed that this is the key function for the success of the attack, and then we will analyze it specifically:
function swapTokenToToken(address _src, address _dest, uint _amount, uint _minPrice, uint _exchangeType, address _exchangeAddress, bytes memory _callData, uint _0xPrice) public payable {
// use this to avoid stack too deep error
address[3] memory orderAddresses = [_exchangeAddress, _src, _dest];if (orderAddresses[1] == KYBER_ETH_ADDRESS) {
require(msg.value >= _amount, “msg.value smaller than amount”);
} else {
require(ERC20(orderAddresses[1]).transferFrom(msg.sender, address(this), _amount), “Not able to withdraw wanted amount”);
}uint fee = takeFee(_amount, orderAddresses[1]);
_amount = sub(_amount, fee);
// [tokensReturned, tokensLeft]
uint[2] memory tokens;
address wrapper;
uint price;
bool success;// at the beggining tokensLeft equals _amount
tokens[1] = _amount;if (_exchangeType == 4) {
if (orderAddresses[1] != KYBER_ETH_ADDRESS) {
ERC20(orderAddresses[1]).approve(address(ERC20_PROXY_0X), _amount);
}(success, tokens[0], ) = takeOrder(orderAddresses, _callData, address(this).balance, _amount);
// either it reverts or order doesn’t exist anymore, we reverts as it was explicitely asked for this exchange
require(success && tokens[0] > 0, “0x transaction failed”);
wrapper = address(_exchangeAddress);
}if (tokens[0] == 0) {
(wrapper, price) = getBestPrice(_amount, orderAddresses[1], orderAddresses[2], _exchangeType);require(price > _minPrice || _0xPrice > _minPrice, “Slippage hit”);
// handle 0x exchange, if equal price, try 0x to use less gas
if (_0xPrice >= price) {
if (orderAddresses[1] != KYBER_ETH_ADDRESS) {
ERC20(orderAddresses[1]).approve(address(ERC20_PROXY_0X), _amount);
}
(success, tokens[0], tokens[1]) = takeOrder(orderAddresses, _callData, address(this).balance, _amount);
// either it reverts or order doesn’t exist anymore
if (success && tokens[0] > 0) {
wrapper = address(_exchangeAddress);
emit Swap(orderAddresses[1], orderAddresses[2], _amount, tokens[0], wrapper);
}
}if (tokens[1] > 0) {
// in case 0x swapped just some amount of tokens and returned everything else
if (tokens[1] != _amount) {
(wrapper, price) = getBestPrice(tokens[1], orderAddresses[1], orderAddresses[2], _exchangeType);
}// in case 0x failed, price on other exchanges still needs to be higher than minPrice
require(price > _minPrice, “Slippage hit onchain price”);
if (orderAddresses[1] == KYBER_ETH_ADDRESS) {
(tokens[0],) = ExchangeInterface(wrapper).swapEtherToToken.value(tokens[1])(tokens[1], orderAddresses[2], uint(-1));
} else {
ERC20(orderAddresses[1]).transfer(wrapper, tokens[1]);if (orderAddresses[2] == KYBER_ETH_ADDRESS) {
tokens[0] = ExchangeInterface(wrapper).swapTokenToEther(orderAddresses[1], tokens[1], uint(-1));
} else {
tokens[0] = ExchangeInterface(wrapper).swapTokenToToken(orderAddresses[1], orderAddresses[2], tokens[1]);
}
}emit Swap(orderAddresses[1], orderAddresses[2], _amount, tokens[0], wrapper);
}
}// return whatever is left in contract
if (address(this).balance > 0) {
msg.sender.transfer(address(this).balance);
}// return if there is any tokens left
if (orderAddresses[2] != KYBER_ETH_ADDRESS) {
if (ERC20(orderAddresses[2]).balanceOf(address(this)) > 0) {
ERC20(orderAddresses[2]).transfer(msg.sender, ERC20(orderAddresses[2]).balanceOf(address(this)));
}
}if (orderAddresses[1] != KYBER_ETH_ADDRESS) {
if (ERC20(orderAddresses[1]).balanceOf(address(this)) > 0) {
ERC20(orderAddresses[1]).transfer(msg.sender, ERC20(orderAddresses[1]).balanceOf(address(this)));
}
}
}
1. In line 5 of the code, you can see whether orderAddresses[1] is a KYBER_ETH_ADDRESS address. Since orderAddresses[1] is a DAI contract address, the transferFrom function will be called directly to transfer the amount of DAI into this book. contract.
2. Next, in the 11th and 12th lines of the code, the fee is calculated by the takeFee function, and the final calculation result is all 0, so no expansion is done here.
3. Since the _exchangeType passed by the attacker is 4, the logic of if (_exchangeType == 4) in line 22 of the code will be followed . In the code, we can see that the takeOrder function is called in this logic , and the attacker’s custom _callData is passed in. Note that this will be the key point of this attack. Next, let’s analyze the takeOrder function:
function takeOrder(address[3] memory _addresses, bytes memory _data, uint _value, uint _amount) private returns(bool, uint, uint) {
bool success;(success, ) = _addresses[0].call.value(_value)(_data);
uint tokensLeft = _amount;
uint tokensReturned = 0;
if (success){
// check how many tokens left from _src
if (_addresses[1] == KYBER_ETH_ADDRESS) {
tokensLeft = address(this).balance;
} else {
tokensLeft = ERC20(_addresses[1]).balanceOf(address(this));
}// check how many tokens are returned
if (_addresses[2] == KYBER_ETH_ADDRESS) {
TokenInterface(WETH_ADDRESS).withdraw(TokenInterface(WETH_ADDRESS).balanceOf(address(this)));
tokensReturned = address(this).balance;
} else {
tokensReturned = ERC20(_addresses[2]).balanceOf(address(this));
}
}return (success, tokensReturned, tokensLeft);
}
4. In the fourth line of the takeOrder function, we can intuitively see that this logic can call the function of the target _addresses[0]. At this time, _addresses[0] is _exchangeAddress, which is the DAI contract address, and the specific The call is the _callData passed in by the attacker, so if the user who holds DAI has authorized the SaverExchange contract in the DAI contract , he can use the passed _callData to call the transferFrom function of the DAI contract to transfer the user’s DAI directly , Can be constructed in _callData.
5. Next, since the returned tokens[0] is 1, we will go through the logic below line 76 in the swapTokenToToken function code block. You can see that the logic is judged by if, and it can work without a doubt.
Analysis and verification
Let us verify whether this process is as we thought through the actions of the attacker:
1. From the records on the chain, it can be seen that the stolen user has historically authorized the SaverExchange contract for DAI, and the transaction hash is as follows:
0xdcf73848022ec1f730d9fdb90f4e8563f0dff48d9191aab19fc51241708eacf0
2. Through the data on the chain, it can be found that the incoming _callData is:
23b872dd //SlowMist// transferFrom 函数签名
000000000000000000000000c001cd7a
370524209626e28eca6abe6cfc09b0e5
0000000000000000000000005bb456cd
09d85156e182d2c7797eb49a43840187
00000000000000000000000000000000
00000000000041a522386d9b95c00000 //SlowMist// 310000e18
It can be seen that 23b872dd is the signature of the transferFrom function.
3. Through the on-chain calling process, it can be seen that the attacker directly calls the transferFrom function of the DAI contract to transfer the 310,000 DAI of the stolen user:
The complete attack process
1. The attacker calls the swapTokenToToken function to pass in _exchangeAddress as the DAI contract address, selects _exchangeType as 4, and puts the attack payload in _callData to pass in.
2. At this time, the logic of _exchangeType == 4 will be followed, which will call the takeOrder function and pass in _callData.
3. The takeOrder function will make a specific call to the incoming _callData, so if the user who holds DAI has authorized the SaverExchange contract in the DAI contract , he can use the incoming _callData to call the transferFrom function of the DAI contract to transfer the user’s DAI Export directly, and it can be constructed in _callData.
4. Through the constructed _callData and the previous user authorized the SaverExchange contract with DAI, the SaverExchange contract can directly transfer the DAI in the user account to the address specified by the attacker by calling the transferFrom function of the DAI contract.
Suggestions
The key to this vulnerability is that an attacker can use the takeOrder function to call any function of the target contract _addresses[0] arbitrarily, and the parameters passed into the takeOrder function are user-controllable, and there are no checks or restrictions on the parameters. Therefore, in order to avoid such problems, it is recommended that the project party use a whitelist strategy to check the parameters such as _callData passed in by the user, or combine the specific business scenarios of the project party to find a better calling method instead of imposing any restrictions. Make random calls.
This vulnerability not only affects users who have authorized the SaverExchange contract through the DAI contract . If the user history has authorized other tokens for the SaverExchange contract, there is a risk that the account token will be arbitrarily transferred out. It is recommended that users who have previously authorized the SaverExchange contract cancel the authorization as soon as possible (it is recommended to use https://approve.sh/ to check the authorization status of the website) to avoid malicious transfer of account assets.
Related reference links are as follows:
https://medium.com/defi-saver/disclosing-a-recently-discovered-exchange-vulnerability-fcd0b61edffe
https://twitter.com/imTokenOfficial/status/1314126579971186688
