The protocol accepts arbitrary values for function parameters and does not protect from re-entrancy opening possibilities to exploit the system.
Vulnerability Detail
There are functions that have many user-controlled values. These values can be fabricated to invoke malicious functions on custom smart contracts.
Here is an example:
Lend for Swivel has many parameters:
/// @notice lend method signature for Swivel
/// @param p principal value according to the MarketPlace's Principals Enum
/// @param u address of an underlying asset
/// @param m maturity (timestamp) of the market
/// @param a array of amounts of underlying tokens lent to each order in the orders array
/// @param y Yield Space Pool for the Illuminate PT in this market
/// @param o array of Swivel orders being filled
/// @param s array of signatures for each order in the orders array
/// @param e flag to indicate if returned funds should be swapped in Yield Space Pool
/// @param premiumSlippage slippage limit, minimum amount to PTs to buy
/// @return uint256 the amount of principal tokens lent out
function lend(
uint8 p,
address u,
uint256 m,
uint256[] memory a,
address y,
Swivel.Order[] calldata o,
Swivel.Components[] calldata s,
bool e,
uint256 premiumSlippage
) external unpaused(u, m, p) returns (uint256) {
The main flow validates the received amount of underlying (end - start balance) and mints the tokens accordingly. However, there exists a branch for premiums:
if (e) {
// Calculate the premium
uint256 premium = IERC20(u).balanceOf(address(this)) -
starting;
// Swap the premium for Illuminate principal tokens
swivelLendPremium(u, m, y, premium, premiumSlippage);
}
e is a user-controlled-value. Let's follow the swivelLendPremium:
/// @notice lends the leftover underlying premium to the Illuminate PT's Yield Space Pool
function swivelLendPremium(
address u,
uint256 m,
address y,
uint256 p,
uint256 slippageTolerance
) internal {
// Lend remaining funds to Illuminate's Yield Space Pool
uint256 swapped = yield(
u,
y,
p,
address(this),
IMarketPlace(marketPlace).token(u, m, 0),
slippageTolerance
);
// Mint the remaining tokens
IERC5095(principalToken(u, m)).authMint(msg.sender, swapped);
}
It swaps premiums and mints the corresponding tokens but let's follow the execution further to the yield function:
/// @notice swaps underlying premium via a Yield Space Pool
/// @dev this method is only used by the Yield, Illuminate and Swivel protocols
/// @param u address of an underlying asset
/// @param y Yield Space Pool for the principal token
/// @param a amount of underlying tokens to lend
/// @param r the receiving address for PTs
/// @param p the principal token in the Yield Space Pool
/// @param m the minimum amount to purchase
/// @return uint256 the amount of tokens sent to the Yield Space Pool
function yield(
address u,
address y,
uint256 a,
address r,
address p,
uint256 m
) internal returns (uint256) {
// Get the starting balance (to verify receipt of tokens)
uint256 starting = IERC20(p).balanceOf(r);
// Get the amount of tokens received for swapping underlying
uint128 returned = IYield(y).sellBasePreview(Cast.u128(a));
// Send the remaining amount to the Yield pool
Safe.transfer(IERC20(u), y, a);
// Lend out the remaining tokens in the Yield pool
IYield(y).sellBase(r, returned);
// Get the ending balance of principal tokens (must be at least starting + returned)
uint256 received = IERC20(p).balanceOf(r) - starting;
// Verify receipt of PTs from Yield Space Pool
if (received <= m) {
revert Exception(11, received, m, address(0), address(0));
}
return received;
}
y is a user-controlled parameter so any contract that conforms to the IYield interface can be passed. m slippage control parameter is also user-controlled so basically, the users are given control to just keep the premiums at this point.
There are lots of functions that give users such control. This becomes especially dangerous when combined with re-entrancy, e.g.:
/// @notice redeem method signature for Sense
/// @param p principal value according to the MarketPlace's Principals Enum
/// @param u address of an underlying asset
/// @param m maturity (timestamp) of the market
/// @param s Sense's maturity is needed to extract the pt address
/// @param a Sense's adapter for this market
/// @return bool true if the redemption was successful
function redeem(
uint8 p,
address u,
uint256 m,
uint256 s,
address a
) external returns (bool)
a Sense's adapter for this market is supplied by the user. The balance after/before is validated, however, it does not account that the malicious contract can re-enter in the middle of execution:
// Get the starting balance to verify the amount received afterward
uint256 starting = IERC20(u).balanceOf(address(this));
// Get the divider from the adapter
ISenseDivider divider = ISenseDivider(ISenseAdapter(a).divider());
// Redeem the tokens from the Sense contract
ISenseDivider(divider).redeem(a, s, amount);
// Get the compounding token that is redeemed by Sense
address compounding = ISenseAdapter(a).target();
// Redeem the compounding token back to the underlying
IConverter(converter).convert(
compounding,
u,
IERC20(compounding).balanceOf(address(this))
);
// Get the amount received
uint256 redeemed = IERC20(u).balanceOf(address(this)) - starting;
Let's say the starting balance is 100 but then the control is given to the malicious contract (ISenseAdapter(a)), it can invoke this function again, and the starting balance will be 100 again. Then the second time this malicious contract sends tokens directly and the same balance after will be accounted for multiple times in holdings.
Impact
The protocol exposes too many unprotected functions and parameters for a crafty user to manipulate the protocol and leak additional value. Input sanitization must be improved before launching the protocol in production.
HonorLt
high
User controlled parameters and re-entrancy
Summary
The protocol accepts arbitrary values for function parameters and does not protect from re-entrancy opening possibilities to exploit the system.
Vulnerability Detail
There are functions that have many user-controlled values. These values can be fabricated to invoke malicious functions on custom smart contracts.
Here is an example:
Lend for Swivel has many parameters:
The main flow validates the received amount of underlying (end - start balance) and mints the tokens accordingly. However, there exists a branch for premiums:
e
is a user-controlled-value. Let's follow theswivelLendPremium
:It swaps premiums and mints the corresponding tokens but let's follow the execution further to the
yield
function:y
is a user-controlled parameter so any contract that conforms to the IYield interface can be passed.m
slippage control parameter is also user-controlled so basically, the users are given control to just keep the premiums at this point.There are lots of functions that give users such control. This becomes especially dangerous when combined with re-entrancy, e.g.:
a Sense's adapter for this market
is supplied by the user. The balance after/before is validated, however, it does not account that the malicious contract can re-enter in the middle of execution:Let's say the starting balance is 100 but then the control is given to the malicious contract (
ISenseAdapter(a)
), it can invoke this function again, and the starting balance will be 100 again. Then the second time this malicious contract sends tokens directly and the same balance after will be accounted for multiple times in holdings.Impact
The protocol exposes too many unprotected functions and parameters for a crafty user to manipulate the protocol and leak additional value. Input sanitization must be improved before launching the protocol in production.
Code Snippet
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L417-L424
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L959-L979
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L939-L946
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L366-L386
Tool used
Manual Review
Recommendation
Do not trust user inputs, add a whitelisted set of addresses or something like that and add a re-entrancy guard to critical functions.
Duplicate of #179