Description
Lines of code
https://github.com/ebtc-protocol/ebtc/blob/2da05f2d1354190eacf64ec256d2c5c5358e5fa2/packages/contracts/contracts/LeverageMacroBase.sol#L241-L243
https://github.com/ebtc-protocol/ebtc/blob/2da05f2d1354190eacf64ec256d2c5c5358e5fa2/packages/contracts/contracts/LeverageMacroBase.sol#L248
https://github.com/ebtc-protocol/ebtc/blob/2da05f2d1354190eacf64ec256d2c5c5358e5fa2/packages/contracts/contracts/LeverageMacroBase.sol#L43-L46
Vulnerability details
LeverageMacroBase.sol has an internal function _doOperation that handles the execution of the CDP operation requested by an user, which can be opening a CDP, adjusting a CDP or closing a CDP.
LeverageMacroBase::_doOperation
function _doOperation(
FlashLoanType flType,
uint256 borrowAmount,
LeverageMacroOperation memory operation,
PostOperationCheck postCheckType,
PostCheckParams memory checkParams,
bytes32 expectedCdpId
) internal {
// Call FL Here, then the stuff below needs to happen inside the FL
if (operation.amountToTransferIn > 0) {
IERC20(operation.tokenToTransferIn).safeTransferFrom(
msg.sender,
address(this),
operation.amountToTransferIn
);
}
// Take eBTC or stETH FlashLoan
if (flType == FlashLoanType.eBTC) {
IERC3156FlashLender(address(borrowerOperations)).flashLoan(
IERC3156FlashBorrower(address(this)),
address(ebtcToken),
borrowAmount,
abi.encode(operation)
);
} else if (flType == FlashLoanType.stETH) {
IERC3156FlashLender(address(activePool)).flashLoan(
IERC3156FlashBorrower(address(this)),
address(stETH),
borrowAmount,
abi.encode(operation)
);
} else {
// No leverage, just do the operation
_handleOperation(operation);
}After handling the operation, the _doOperation function performs post-operation checks to ensure that the CDP position values, such as debt and collateral, are as expected post-operation execution.
/**
* POST CALL CHECK FOR CREATION
*/
if (postCheckType == PostOperationCheck.openCdp) {
// Check for param details
ICdpManagerData.Cdp memory cdpInfo = cdpManager.Cdps(expectedCdpId);
_doCheckValueType(checkParams.expectedDebt, cdpInfo.debt);
_doCheckValueType(checkParams.expectedCollateral, cdpInfo.coll);
require(
cdpInfo.status == checkParams.expectedStatus,
"!LeverageMacroReference: openCDP status check"
);
}
// Update CDP, Ensure the stats are as intended
if (postCheckType == PostOperationCheck.cdpStats) {
ICdpManagerData.Cdp memory cdpInfo = cdpManager.Cdps(checkParams.cdpId);
_doCheckValueType(checkParams.expectedDebt, cdpInfo.debt);
_doCheckValueType(checkParams.expectedCollateral, cdpInfo.coll);
require(
cdpInfo.status == checkParams.expectedStatus,
"!LeverageMacroReference: adjustCDP status check"
);
}
// Post check type: Close, ensure it has the status we want
if (postCheckType == PostOperationCheck.isClosed) {
ICdpManagerData.Cdp memory cdpInfo = cdpManager.Cdps(checkParams.cdpId);
require(
cdpInfo.status == checkParams.expectedStatus,
"!LeverageMacroReference: closeCDP status check"
);
}
After performing the post-operation checks, there is a selection statement that checks if willSweep has been set to true and if it evaluates to true, the _doOperation function calls sweepToCaller to ensure that any dust that has been left over from the performed operation is transferred back to the user.
// Sweep here if it's Reference, do not if it's delegate
if (willSweep) {
@> sweepToCaller();
}The sweepToCaller function calls another internal function _assertOwner, which checks who is the caller of the sweepToCaller function and compares it against the owner of the LeverageZapRouterBase contract that has been initialized during the contract construction.
LeverageMacroBase::sweepToCaller
/// @notice Sweep away tokens if they are stuck here
function sweepToCaller() public {
@> _assertOwner();
/**
* SWEEP TO CALLER *
*/
// Safe unchecked because known tokens
uint256 ebtcBal = ebtcToken.balanceOf(address(this));
uint256 collateralBal = stETH.sharesOf(address(this));
if (ebtcBal > 0) {
ebtcToken.transfer(msg.sender, ebtcBal);
}
if (collateralBal > 0) {
stETH.transferShares(msg.sender, collateralBal);
}
}
LeverageMacroBase::_assertOwner
function _assertOwner() internal {
// Reference will compare to variable,
@> require(owner() == msg.sender, "Must be owner");
}
LeverageZapRouterBase::constructor
abstract contract LeverageZapRouterBase is ZapRouterBase, LeverageMacroBase, ReentrancyGuard, IEbtcLeverageZapRouter {
using SafeERC20 for IERC20;
uint256 internal constant PRECISION = 1e18;
uint256 internal constant BPS = 10000;
address public immutable theOwner;
address public immutable DEX;
uint256 public immutable zapFeeBPS;
address public immutable zapFeeReceiver;
constructor(
IEbtcLeverageZapRouter.DeploymentParams memory params
)
ZapRouterBase(
params.borrowerOperations,
IERC20(params.wstEth),
IERC20(params.weth),
IStETH(params.stEth)
)
LeverageMacroBase(
params.borrowerOperations,
params.activePool,
params.cdpManager,
params.ebtc,
params.stEth,
params.sortedCdps,
false // Do not sweep
)
{
if (params.zapFeeBPS > 0) {
require(params.zapFeeReceiver != address(0));
}
@> theOwner = params.owner;
DEX = params.dex;
zapFeeBPS = params.zapFeeBPS;
zapFeeReceiver = params.zapFeeReceiver;
// Infinite Approvals @TODO: do these stay at max for each token?
ebtcToken.approve(address(borrowerOperations), type(uint256).max);
stETH.approve(address(borrowerOperations), type(uint256).max);
stETH.approve(address(activePool), type(uint256).max);
stEth.approve(address(wstEth), type(uint256).max);
}However, given that any user can perform operations such as opening a CDP, adjusting a CDP and closing a CDP, it is unsafe to assume that the caller of the sweepToCaller function will be equal to the owner address initialized during the LeverageZapRouterBase construction.
Impact
If willSweep is set to true, all main functions - openCdp, adjustCdp, closeCdp - will revert because msg.sender can be any arbitrary EOA address and would not necessarily be equal to the owner address of the contract.
Although in the specific case of LeverageZapRouterBase, the willSweep variable is initialized to false, the overall implementation is flawed. The sweepToCaller function can only be called by the owner of the LeverageZapRouterBase and no one else, effectively making it a sweepToOwner instead of a sweepToCaller.
Proof of Concept
Tools Used
Manual Review
Recommended Mitigation Steps
Change the sweepToCaller function's visibility to private to ensure it cannot be called by anyone other than the contract itself and remove the _assertOwner call, so that it can properly transfer the remaining dust to any arbitrary EOA address and not just the owner address.
/// @notice Sweep away tokens if they are stuck here
- function sweepToCaller() public {
+ function sweepToCaller() private {
- _assertOwner();
/**
* SWEEP TO CALLER *
*/
// Safe unchecked because known tokens
uint256 ebtcBal = ebtcToken.balanceOf(address(this));
uint256 collateralBal = stETH.sharesOf(address(this));
if (ebtcBal > 0) {
ebtcToken.transfer(msg.sender, ebtcBal);
}
if (collateralBal > 0) {
stETH.transferShares(msg.sender, collateralBal);
}
}Assessed type
DoS