- Added reentrancy guard on repayWithCollateral() and test.

This commit is contained in:
eboado 2020-09-09 13:06:46 +02:00
parent 56ddeceb94
commit 37a9c7ad88
4 changed files with 61 additions and 6 deletions

View File

@ -51,6 +51,8 @@ contract LendingPool is ReentrancyGuard, VersionedInitializable, ILendingPool {
address[] internal _reservesList; address[] internal _reservesList;
bool internal _flashLiquidationLocked;
/** /**
* @dev only lending pools configurator can use functions affected by this modifier * @dev only lending pools configurator can use functions affected by this modifier
**/ **/
@ -474,7 +476,10 @@ contract LendingPool is ReentrancyGuard, VersionedInitializable, ILendingPool {
uint256 principalAmount, uint256 principalAmount,
address receiver, address receiver,
bytes calldata params bytes calldata params
) external override nonReentrant { ) external override {
require(!_flashLiquidationLocked, "REENTRANCY_NOT_ALLOWED");
_flashLiquidationLocked = true;
address liquidationManager = _addressesProvider.getLendingPoolLiquidationManager(); address liquidationManager = _addressesProvider.getLendingPoolLiquidationManager();
//solium-disable-next-line //solium-disable-next-line
@ -496,6 +501,8 @@ contract LendingPool is ReentrancyGuard, VersionedInitializable, ILendingPool {
if (returnCode != 0) { if (returnCode != 0) {
revert(string(abi.encodePacked(returnMessage))); revert(string(abi.encodePacked(returnMessage)));
} }
_flashLiquidationLocked = false;
} }
/** /**

View File

@ -37,6 +37,9 @@ contract LendingPoolLiquidationManager is ReentrancyGuard, VersionedInitializabl
using ReserveConfiguration for ReserveConfiguration.Map; using ReserveConfiguration for ReserveConfiguration.Map;
using UserConfiguration for UserConfiguration.Map; using UserConfiguration for UserConfiguration.Map;
// IMPORTANT The storage layout of the LendingPool is reproduced here because this contract
// is gonna be used through DELEGATECALL
LendingPoolAddressesProvider internal addressesProvider; LendingPoolAddressesProvider internal addressesProvider;
mapping(address => ReserveLogic.ReserveData) internal reserves; mapping(address => ReserveLogic.ReserveData) internal reserves;
@ -44,6 +47,8 @@ contract LendingPoolLiquidationManager is ReentrancyGuard, VersionedInitializabl
address[] internal reservesList; address[] internal reservesList;
bool internal _flashLiquidationLocked;
uint256 internal constant LIQUIDATION_CLOSE_FACTOR_PERCENT = 5000; uint256 internal constant LIQUIDATION_CLOSE_FACTOR_PERCENT = 5000;
/** /**

View File

@ -4,11 +4,13 @@ pragma solidity ^0.6.8;
import {MintableERC20} from '../tokens/MintableERC20.sol'; import {MintableERC20} from '../tokens/MintableERC20.sol';
import {ILendingPoolAddressesProvider} from '../../interfaces/ILendingPoolAddressesProvider.sol'; import {ILendingPoolAddressesProvider} from '../../interfaces/ILendingPoolAddressesProvider.sol';
import {ISwapAdapter} from '../../interfaces/ISwapAdapter.sol'; import {ISwapAdapter} from '../../interfaces/ISwapAdapter.sol';
import {ILendingPool} from "../../interfaces/ILendingPool.sol";
import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol';
contract MockSwapAdapter is ISwapAdapter { contract MockSwapAdapter is ISwapAdapter {
uint256 amountToReturn; uint256 internal _amountToReturn;
bool internal _tryReentrancy;
ILendingPoolAddressesProvider public addressesProvider; ILendingPoolAddressesProvider public addressesProvider;
event Swapped(address fromAsset, address toAsset, uint256 fromAmount, uint256 receivedAmount); event Swapped(address fromAsset, address toAsset, uint256 fromAmount, uint256 receivedAmount);
@ -18,7 +20,11 @@ contract MockSwapAdapter is ISwapAdapter {
} }
function setAmountToReturn(uint256 amount) public { function setAmountToReturn(uint256 amount) public {
amountToReturn = amount; _amountToReturn = amount;
}
function setTryReentrancy(bool tryReentrancy) public {
_tryReentrancy = tryReentrancy;
} }
function executeOperation( function executeOperation(
@ -30,10 +36,21 @@ contract MockSwapAdapter is ISwapAdapter {
) external override { ) external override {
params; params;
IERC20(assetToSwapFrom).transfer(address(1), amountToSwap); // We don't want to keep funds here IERC20(assetToSwapFrom).transfer(address(1), amountToSwap); // We don't want to keep funds here
MintableERC20(assetToSwapTo).mint(amountToReturn); MintableERC20(assetToSwapTo).mint(_amountToReturn);
IERC20(assetToSwapTo).approve(fundsDestination, amountToReturn); IERC20(assetToSwapTo).approve(fundsDestination, _amountToReturn);
emit Swapped(assetToSwapFrom, assetToSwapTo, amountToSwap, amountToReturn); if (_tryReentrancy) {
ILendingPool(fundsDestination).repayWithCollateral(
assetToSwapFrom,
assetToSwapTo,
address(1), // Doesn't matter, we just want to test the reentrancy
1 ether, // Same
address(1), // Same
"0x"
);
}
emit Swapped(assetToSwapFrom, assetToSwapTo, amountToSwap, _amountToReturn);
} }
function burnAsset(IERC20 asset, uint256 amount) public { function burnAsset(IERC20 asset, uint256 amount) public {

View File

@ -67,12 +67,38 @@ makeSuite('LendingPool. repayWithCollateral()', (testEnv: TestEnv) => {
await pool.connect(user.signer).borrow(dai.address, amountToBorrow, 2, 0); await pool.connect(user.signer).borrow(dai.address, amountToBorrow, 2, 0);
}); });
it('It is not possible to do reentrancy on repayWithCollateral()', async () => {
const {pool, weth, dai, users, mockSwapAdapter, oracle} = testEnv;
const user = users[1];
const amountToRepay = parseEther('10');
await waitForTx(await mockSwapAdapter.setTryReentrancy(true))
await mockSwapAdapter.setAmountToReturn(amountToRepay);
await expect(
pool
.connect(user.signer)
.repayWithCollateral(
weth.address,
dai.address,
user.address,
amountToRepay,
mockSwapAdapter.address,
'0x'
)
).to.be.revertedWith("FAILED_REPAY_WITH_COLLATERAL")
});
it('User 2 tries to repay his DAI Variable loan using his WETH collateral. First half the amount, after that, the rest', async () => { it('User 2 tries to repay his DAI Variable loan using his WETH collateral. First half the amount, after that, the rest', async () => {
const {pool, weth, dai, users, mockSwapAdapter, oracle} = testEnv; const {pool, weth, dai, users, mockSwapAdapter, oracle} = testEnv;
const user = users[1]; const user = users[1];
const amountToRepay = parseEther('10'); const amountToRepay = parseEther('10');
await waitForTx(await mockSwapAdapter.setTryReentrancy(false))
const {userData: wethUserDataBefore} = await getContractsData( const {userData: wethUserDataBefore} = await getContractsData(
weth.address, weth.address,
user.address, user.address,