From 24570c90c9116bf431a14bc8db21910a7e4ae3e0 Mon Sep 17 00:00:00 2001 From: The3D Date: Fri, 25 Jun 2021 14:04:19 +0200 Subject: [PATCH] feat: added check on the permission admin validity on the borrow/deposit/liquidate actions --- contracts/interfaces/IPermissionManager.sol | 7 ++++ contracts/misc/PermissionedWETHGateway.sol | 33 +++++++++++++------ .../configuration/PermissionManager.sol | 5 +++ .../lendingpool/PermissionedLendingPool.sol | 33 ++++++++++++++----- .../protocol/libraries/helpers/Errors.sol | 11 ++++--- .../base/PermissionedDebtTokenBase.sol | 2 +- 6 files changed, 66 insertions(+), 25 deletions(-) diff --git a/contracts/interfaces/IPermissionManager.sol b/contracts/interfaces/IPermissionManager.sol index 6e983baa..f2a64ab6 100644 --- a/contracts/interfaces/IPermissionManager.sol +++ b/contracts/interfaces/IPermissionManager.sol @@ -72,4 +72,11 @@ interface IPermissionManager { * @return the address of the permissionAdmin of the user **/ function getUserPermissionAdmin(address user) external view returns (address); + + /** + * @dev Used to query if the permission admin of a certain user is valid, + * @param user The address of the user + * @return true if the permission admin of a certain user is valid, false otherwise + **/ + function isUserPermissionAdminValid(address user) external view returns (bool); } diff --git a/contracts/misc/PermissionedWETHGateway.sol b/contracts/misc/PermissionedWETHGateway.sol index 782e3a0b..8d83eb0d 100644 --- a/contracts/misc/PermissionedWETHGateway.sol +++ b/contracts/misc/PermissionedWETHGateway.sol @@ -37,7 +37,10 @@ contract PermissionedWETHGateway is WETHGateway { ) public payable override { ILendingPool pool = ILendingPool(lendingPool); - require(_isInRole(msg.sender, DataTypes.Roles.DEPOSITOR, pool), Errors.USER_UNAUTHORIZED); + require( + _isInRoleAndValidPermissionAdmin(msg.sender, DataTypes.Roles.DEPOSITOR, pool), + Errors.PLP_USER_UNAUTHORIZED + ); super.depositETH(lendingPool, onBehalfOf, referralCode); } @@ -57,21 +60,31 @@ contract PermissionedWETHGateway is WETHGateway { ) public payable override { ILendingPool pool = ILendingPool(lendingPool); - require(_isInRole(msg.sender, DataTypes.Roles.BORROWER, pool), Errors.USER_UNAUTHORIZED); + require(_isInRole(msg.sender, DataTypes.Roles.BORROWER, pool), Errors.PLP_USER_UNAUTHORIZED); super.repayETH(lendingPool, amount, rateMode, onBehalfOf); } - - function _isInRole(address user, DataTypes.Roles role, ILendingPool pool) - internal - view - returns (bool) - { + function _getPermissionManager(ILendingPool pool) internal view returns (IPermissionManager) { ILendingPoolAddressesProvider provider = ILendingPoolAddressesProvider(pool.getAddressesProvider()); - IPermissionManager manager = - IPermissionManager(provider.getAddress(keccak256('PERMISSION_MANAGER'))); + return IPermissionManager(provider.getAddress(keccak256('PERMISSION_MANAGER'))); + } + function _isInRole( + address user, + DataTypes.Roles role, + ILendingPool pool + ) internal view returns (bool) { + IPermissionManager manager = _getPermissionManager(pool); return manager.isInRole(user, uint256(role)); } + + function _isInRoleAndValidPermissionAdmin( + address user, + DataTypes.Roles role, + ILendingPool pool + ) internal view returns (bool) { + IPermissionManager manager = _getPermissionManager(pool); + return manager.isInRole(user, uint256(role)) && manager.isUserPermissionAdminValid(user); + } } diff --git a/contracts/protocol/configuration/PermissionManager.sol b/contracts/protocol/configuration/PermissionManager.sol index 4a3156ad..2ffd8d81 100644 --- a/contracts/protocol/configuration/PermissionManager.sol +++ b/contracts/protocol/configuration/PermissionManager.sol @@ -175,4 +175,9 @@ contract PermissionManager is IPermissionManager, Ownable { function getUserPermissionAdmin(address user) external view override returns (address) { return _users[user].permissionAdmin; } + + ///@inheritdoc IPermissionManager + function isUserPermissionAdminValid(address user) external view override returns (bool) { + return _permissionsAdmins[_users[user].permissionAdmin] > 0; + } } diff --git a/contracts/protocol/lendingpool/PermissionedLendingPool.sol b/contracts/protocol/lendingpool/PermissionedLendingPool.sol index 6b8fa64b..044d7329 100644 --- a/contracts/protocol/lendingpool/PermissionedLendingPool.sol +++ b/contracts/protocol/lendingpool/PermissionedLendingPool.sol @@ -20,29 +20,34 @@ contract PermissionedLendingPool is LendingPool { require( _isInRole(user, DataTypes.Roles.DEPOSITOR) && ((user == msg.sender) || _isInRole(msg.sender, DataTypes.Roles.DEPOSITOR)), - Errors.DEPOSITOR_UNAUTHORIZED + Errors.PLP_DEPOSITOR_UNAUTHORIZED ); _; } + modifier onlyValidPermissionAdmin(address user) { + require(_permissionAdminValid(user), Errors.PLP_INVALID_PERMISSION_ADMIN); + _; + } + modifier onlyBorrowers(address user) { require( _isInRole(user, DataTypes.Roles.BORROWER) && ((user == msg.sender) || _isInRole(msg.sender, DataTypes.Roles.BORROWER)), - Errors.BORROWER_UNAUTHORIZED + Errors.PLP_BORROWER_UNAUTHORIZED ); _; } modifier onlyLiquidators { - require(_isInRole(msg.sender, DataTypes.Roles.LIQUIDATOR), Errors.LIQUIDATOR_UNAUTHORIZED); + require(_isInRole(msg.sender, DataTypes.Roles.LIQUIDATOR), Errors.PLP_LIQUIDATOR_UNAUTHORIZED); _; } modifier onlyStableRateManagers { require( _isInRole(msg.sender, DataTypes.Roles.STABLE_RATE_MANAGER), - Errors.CALLER_NOT_STABLE_RATE_MANAGER + Errors.PLP_CALLER_NOT_STABLE_RATE_MANAGER ); _; } @@ -63,7 +68,7 @@ contract PermissionedLendingPool is LendingPool { uint256 amount, address onBehalfOf, uint16 referralCode - ) public virtual override onlyDepositors(onBehalfOf) { + ) public virtual override onlyDepositors(onBehalfOf) onlyValidPermissionAdmin(onBehalfOf) { super.deposit(asset, amount, onBehalfOf, referralCode); } @@ -107,7 +112,7 @@ contract PermissionedLendingPool is LendingPool { uint256 interestRateMode, uint16 referralCode, address onBehalfOf - ) public virtual override onlyBorrowers(onBehalfOf) { + ) public virtual override onlyBorrowers(onBehalfOf) onlyValidPermissionAdmin(onBehalfOf) { super.borrow(asset, amount, interestRateMode, referralCode, onBehalfOf); } @@ -142,6 +147,7 @@ contract PermissionedLendingPool is LendingPool { virtual override onlyBorrowers(msg.sender) + onlyValidPermissionAdmin(msg.sender) { super.swapBorrowRateMode(asset, rateMode); } @@ -174,6 +180,7 @@ contract PermissionedLendingPool is LendingPool { virtual override onlyDepositors(msg.sender) + onlyValidPermissionAdmin(msg.sender) { super.setUserUseReserveAsCollateral(asset, useAsCollateral); } @@ -195,7 +202,7 @@ contract PermissionedLendingPool is LendingPool { address user, uint256 debtToCover, bool receiveAToken - ) public virtual override onlyLiquidators { + ) public virtual override onlyLiquidators onlyValidPermissionAdmin(msg.sender) { super.liquidationCall(collateralAsset, debtAsset, user, debtToCover, receiveAToken); } @@ -228,9 +235,11 @@ contract PermissionedLendingPool is LendingPool { //validating modes for (uint256 i = 0; i < modes.length; i++) { if (modes[i] == uint256(DataTypes.InterestRateMode.NONE)) { - require(_isInRole(msg.sender, DataTypes.Roles.BORROWER), Errors.BORROWER_UNAUTHORIZED); + require(_isInRole(msg.sender, DataTypes.Roles.BORROWER), Errors.PLP_BORROWER_UNAUTHORIZED); + require(_permissionAdminValid(msg.sender), Errors.PLP_INVALID_PERMISSION_ADMIN); } else { - require(_isInRole(onBehalfOf, DataTypes.Roles.BORROWER), Errors.BORROWER_UNAUTHORIZED); + require(_isInRole(onBehalfOf, DataTypes.Roles.BORROWER), Errors.PLP_BORROWER_UNAUTHORIZED); + require(_permissionAdminValid(onBehalfOf), Errors.PLP_INVALID_PERMISSION_ADMIN); } } super.flashLoan(receiverAddress, assets, amounts, modes, onBehalfOf, params, referralCode); @@ -267,4 +276,10 @@ contract PermissionedLendingPool is LendingPool { uint256(role) ); } + + function _permissionAdminValid(address user) internal view returns (bool) { + return + IPermissionManager(_addressesProvider.getAddress(PERMISSION_MANAGER)) + .isUserPermissionAdminValid(user); + } } diff --git a/contracts/protocol/libraries/helpers/Errors.sol b/contracts/protocol/libraries/helpers/Errors.sol index 5730b3fc..399ea06d 100644 --- a/contracts/protocol/libraries/helpers/Errors.sol +++ b/contracts/protocol/libraries/helpers/Errors.sol @@ -103,11 +103,12 @@ library Errors { string public constant LP_NOT_CONTRACT = '78'; string public constant SDT_STABLE_DEBT_OVERFLOW = '79'; string public constant SDT_BURN_EXCEEDS_BALANCE = '80'; - string public constant DEPOSITOR_UNAUTHORIZED = '81'; - string public constant BORROWER_UNAUTHORIZED = '82'; - string public constant LIQUIDATOR_UNAUTHORIZED = '83'; - string public constant CALLER_NOT_STABLE_RATE_MANAGER = '84'; - string public constant USER_UNAUTHORIZED = '85'; + string public constant PLP_DEPOSITOR_UNAUTHORIZED = '81'; + string public constant PLP_BORROWER_UNAUTHORIZED = '82'; + string public constant PLP_LIQUIDATOR_UNAUTHORIZED = '83'; + string public constant PLP_CALLER_NOT_STABLE_RATE_MANAGER = '84'; + string public constant PLP_USER_UNAUTHORIZED = '85'; + string public constant PLP_INVALID_PERMISSION_ADMIN = '86'; enum CollateralManagerErrors { NO_ERROR, diff --git a/contracts/protocol/tokenization/base/PermissionedDebtTokenBase.sol b/contracts/protocol/tokenization/base/PermissionedDebtTokenBase.sol index c095280d..55bbae28 100644 --- a/contracts/protocol/tokenization/base/PermissionedDebtTokenBase.sol +++ b/contracts/protocol/tokenization/base/PermissionedDebtTokenBase.sol @@ -29,7 +29,7 @@ abstract contract PermissionedDebtTokenBase is DebtTokenBase require( permissionManager.isInRole(_msgSender(), uint256(DataTypes.Roles.BORROWER)), - Errors.BORROWER_UNAUTHORIZED + Errors.PLP_BORROWER_UNAUTHORIZED ); _; }