Arbitrary External Call Injection via 1inch Swap Data
Description
The _call1InchSwap function performs a low-level call to the oneInchRouter with arbitrary calldata provided by the user. While the router address is set during initialization, the calldata is completely user-controlled and not validated. This allows the owner to craft malicious calldata that could call any function on the 1inch router, potentially including functions that transfer tokens to arbitrary addresses or interact with other contracts in unintended ways. The lack of function selector validation means the contract trusts that the provided calldata will always call the swap function.
Risk
Likelihood:
Impact:
-
An attacker could drain all approved tokens from the contract by calling unintended functions on the 1inch router (e.g., transferFrom or similar logic exposed by the router).
-
Manipulation of the swap process to benefit the attacker at the expense of the protocol's positions.
Proof of Concept
bytes memory maliciousCalldata = abi.encodeWithSignature(
"unoswapTo(address,address,uint256,uint256,address)",
collateralToken,
attackerAddress,
amount,
minReturn,
debtToken
);
stratax.createLeveragedPosition(
flashLoanToken,
flashLoanAmount,
collateralAmount,
borrowToken,
borrowAmount,
maliciousCalldata,
minReturnAmount
);
Recommended Mitigation
function _call1InchSwap(bytes memory _swapParams, address _asset, uint256 _minReturnAmount)
internal
returns (uint256 returnAmount)
{
+ // Validate the function selector
+ bytes4 selector;
+ assembly {
+ selector := mload(add(_swapParams, 32))
+ }
+ // Only allow specific swap functions
+ require(
+ selector == IAggregationRouter.swap.selector ||
+ selector == bytes4(keccak256("unoswap(address,uint256,uint256,uint256[])")) ||
+ selector == bytes4(keccak256("unoswapTo(address,uint256,uint256,uint256[],address)")),
+ "Invalid swap function"
+ );
// Execute the 1inch swap using low-level call with the calldata from the API
(bool success, bytes memory result) = address(oneInchRouter).call(_swapParams);
require(success, "1inch swap failed");
// Decode the return amount from the swap
if (result.length > 0) {
(returnAmount,) = abi.decode(result, (uint256, uint256));
} else {
// If no return data, check balance
returnAmount = IERC20(_asset).balanceOf(address(this));
}
// Sanity check
require(returnAmount >= _minReturnAmount, "Insufficient return amount from swap");
return returnAmount;
}
Missing Staleness Check in Oracle Price Feeds
Description
The StrataxOracle contract fetches prices from Chainlink oracles but does not validate the freshness of the price data. The latestRoundData() function returns an updatedAt timestamp that should be checked to ensure the price is not stale. During periods of high volatility or oracle issues, stale prices could lead to incorrect leverage calculations, potentially allowing positions to be created with excessive leverage or unwound at unfavorable rates.
Risk
Likelihood:
Impact:
-
Creation of overleveraged positions that immediately become liquidatable.
-
Incorrect calculation of collateral requirements leading to bad debt.
-
Manipulation opportunities where attackers wait for stale prices to execute favorable trades.
-
Unwinding positions at incorrect exchange rates causing loss of funds.
Proof of Concept
TradeDetails memory details = TradeDetails({
collateralToken: ETH,
borrowToken: USDC,
desiredLeverage: 30000,
collateralAmount: 10 ether,
collateralTokenPrice: 0,
borrowTokenPrice: 0,
collateralTokenDec: 18,
borrowTokenDec: 6
});
(uint256 flashLoan, uint256 borrowAmount) = stratax.calculateOpenParams(details);
Recommended Mitigation
function getPrice(address _token) public view returns (uint256 price) {
address priceFeedAddress = priceFeeds[_token];
require(priceFeedAddress != address(0), "Price feed not set for token");
AggregatorV3Interface priceFeed = AggregatorV3Interface(priceFeedAddress);
- (, int256 answer,,,) = priceFeed.latestRoundData();
- require(answer > 0, "Invalid price from oracle");
+ (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) =
+ priceFeed.latestRoundData();
+ require(answer > 0, "Invalid price from oracle");
+ require(updatedAt != 0, "Round not complete");
+ require(answeredInRound >= roundId, "Stale price");
+ // Check if price is stale (more than 1 hour old)
+ uint256 stalePriceThreshold = 3600; // 1 hour
+ require(block.timestamp - updatedAt <= stalePriceThreshold, "Price data is stale");
price = uint256(answer);
}
Incorrect Collateral Withdrawal Calculation in Unwind Operation
Description
In the _executeUnwindOperation function, the collateral withdrawal calculation uses liquidationThreshold instead of LTV, and divides by it rather than using it to calculate the necessary collateral. This results in the calculation attempting to withdraw significantly less collateral than necessary to cover the repaid debt (or vice-versa depending on framing, but analysis shows incorrect formula). The formula divides by liqThreshold, which amplifies or reduces the withdrawal amount incorrectly relative to the actual debt value needed.
Risk
Likelihood:
Impact:
-
The contract may attempt to withdraw an incorrect amount of collateral (likely less than needed due to division by a large integer treated as a small fraction if misinterpreted, or just logically wrong formula).
-
If insufficient collateral is withdrawn, the swap to repay the flash loan may fail due to insufficient funds, causing the transaction to revert ("Insufficient funds to repay flash loan").
-
Users may be unable to close positions or may have excess collateral locked.
Proof of Concept
Recommended Mitigation
// Step 2: Calculate and withdraw only the collateral that backed the repaid debt
uint256 withdrawnAmount;
{
// Get LTV from Aave for the collateral token
- (,, uint256 liqThreshold,,,,,,,) =
+ (, uint256 ltv,,,,,,,,) =
aaveDataProvider.getReserveConfigurationData(unwindParams.collateralToken);
+ require(ltv > 0, "Asset not usable as collateral");
// Get prices and decimals
uint256 debtTokenPrice = IStrataxOracle(strataxOracle).getPrice(_asset);
uint256 collateralTokenPrice = IStrataxOracle(strataxOracle).getPrice(unwindParams.collateralToken);
require(debtTokenPrice > 0 && collateralTokenPrice > 0, "Invalid prices");
- // Calculate collateral to withdraw: (debtAmount * debtPrice * collateralDec * LTV_PRECISION) / (collateralPrice * debtDec * ltv)
- uint256 collateralToWithdraw = (
- _amount * debtTokenPrice * (10 ** IERC20(unwindParams.collateralToken).decimals()) * LTV_PRECISION
- ) / (collateralTokenPrice * (10 ** IERC20(_asset).decimals()) * liqThreshold);
+ // Calculate value of debt in collateral terms
+ uint256 debtValueInCollateral = (
+ _amount * debtTokenPrice * (10 ** IERC20(unwindParams.collateralToken).decimals())
+ ) / (collateralTokenPrice * (10 ** IERC20(_asset).decimals()));
+ // Add 5% buffer for slippage and fees
+ uint256 collateralToWithdraw = (debtValueInCollateral * 10500) / 10000;
withdrawnAmount = aavePool.withdraw(unwindParams.collateralToken, collateralToWithdraw, address(this));
}
Flash Loan Callback Missing Reentrancy Protection
Description
The executeOperation function, which serves as the flash loan callback, lacks reentrancy protection. While it checks that msg.sender is the Aave pool and initiator is the contract itself, it does not use a nonReentrant modifier. During the swap execution in _call1InchSwap, the 1inch router (or a malicious implementation if swapped via a proxy) could call back into the contract, potentially manipulating state or draining funds. The contract performs external calls without following the checks-effects-interactions pattern.
Risk
Likelihood:
Impact:
-
Manipulation of contract state during flash loan execution.
-
Potential to drain funds by re-entering during the 1inch swap when tokens are approved but not yet swapped.
-
Bypassing health factor checks by manipulating position data mid-execution.
Proof of Concept
contract Malicious1inchRouter {
IStratax stratax;
function swap(bytes calldata data) external returns (uint256, uint256) {
stratax.createLeveragedPosition(
collateralToken,
flashAmount,
0,
borrowToken,
borrowAmount,
maliciousSwapData,
0
);
stratax.unwindPosition(
collateralToken,
withdrawAmount,
debtToken,
debtAmount,
swapData,
minReturn
);
return (expectedAmount, spentAmount);
}
}
Recommended Mitigation
// Add OpenZeppelin's ReentrancyGuard
import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
- contract Stratax is Initializable {
+ contract Stratax is Initializable, ReentrancyGuard {
// Add nonReentrant modifier to all external functions
function executeOperation(
address _asset,
uint256 _amount,
uint256 _premium,
address _initiator,
bytes calldata _params
- ) external returns (bool) {
+ ) external nonReentrant returns (bool) {
require(msg.sender == address(aavePool), "Caller must be Aave Pool");
require(_initiator == address(this), "Initiator must be this contract");
// Rest of function...
}
function createLeveragedPosition(
// ... params ...
- ) public onlyOwner {
+ ) public onlyOwner nonReentrant {
// Function implementation...
}
function unwindPosition(
// ... params ...
- ) external onlyOwner {
+ ) external onlyOwner nonReentrant {
// Function implementation...
}
}
No Validation of Oracle Price Feed Addresses
Description
The StrataxOracle contract allows the owner to set any address as a price feed without validating that it is a legitimate Chainlink oracle contract. The only validation present is checking that decimals() returns 8, which is easily spoofable by any malicious contract.
Risk
Likelihood:
Impact:
-
A malicious owner could set fake price feeds that return manipulated prices to create bad debt or steal funds.
-
Accidentally setting the wrong oracle address could lead to incorrect position calculations or transaction failures.
-
Enables price manipulation attacks by controlling the price feed.
Proof of Concept
contract FakePriceFeed {
function decimals() external pure returns (uint8) {
return 8;
}
function latestRoundData() external view returns (
uint80 roundId,
int256 answer,
uint256 startedAt,
uint256 updatedAt,
uint80 answeredInRound
) {
return (1, 1000000000000, block.timestamp, block.timestamp, 1);
}
}
strataxOracle.setPriceFeed(WETH, address(fakePriceFeed));
Recommended Mitigation
// Maintain a whitelist of valid Chainlink oracles
+ mapping(address => bool) public validPriceFeeds;
+ function addValidPriceFeed(address _priceFeed) external onlyOwner {
+ // Verify it's a Chainlink oracle by checking multiple functions
+ AggregatorV3Interface priceFeed = AggregatorV3Interface(_priceFeed);
+ // Check it implements required functions
+ require(priceFeed.decimals() == 8, "Must have 8 decimals");
+ // Verify it returns valid data
+ (uint80 roundId, int256 answer, , uint256 updatedAt, ) = priceFeed.latestRoundData();
+ require(roundId > 0, "Invalid round ID");
+ require(answer > 0, "Invalid price");
+ require(updatedAt > 0 && updatedAt <= block.timestamp, "Invalid timestamp");
+ // Check description is not empty (Chainlink oracles have descriptions)
+ string memory description = priceFeed.description();
+ require(bytes(description).length > 0, "Invalid description");
+ validPriceFeeds[_priceFeed] = true;
+ emit ValidPriceFeedAdded(_priceFeed);
+ }
function _setPriceFeed(address _token, address _priceFeed) internal {
require(_token != address(0), "Invalid token address");
require(_priceFeed != address(0), "Invalid price feed address");
+ require(validPriceFeeds[_priceFeed], "Price feed not whitelisted");
AggregatorV3Interface priceFeed = AggregatorV3Interface(_priceFeed);
require(priceFeed.decimals() == 8, "Price feed must have 8 decimals");
priceFeeds[_token] = _priceFeed;
}
