Omniscia Bonq Audit
stability-pool Manual Review Findings
stability-pool Manual Review Findings
STA-01M: Centralized Control of Sensitive Variables
| Type | Severity | Location |
|---|---|---|
| Centralization Concern |  | stability-pool.sol:L281-L284, L300-L303 |
Description:
The referenced router and bonqToken setter functions are invoke-able by the owner and adjust highly sensitive contract variables.
Example:
281/// @dev sets the BONQ token contract282function setBONQToken(address _bonqTokenAddress) external onlyOwner {283 bonqToken = IERC20(_bonqTokenAddress);284}285
286/// @dev sets amount of BONQ per minute for rewards287function setBONQPerMinute(uint256 _bonqPerMinute) external override onlyOwner {288 bonqPerMinute = _bonqPerMinute;289 emit BONQPerMinuteUpdated(bonqPerMinute);290}291
292/// @dev sets total amount of BONQ to be rewarded (pays per minute until reaches the amount rewarded)293function setBONQAmountForRewards(uint256 _amountForRewards) external override onlyOwner {294 require(bonqToken.balanceOf(address(this)) >= _amountForRewards, "6d94f amount must fit the balance");295 _triggerBONQdistribution();296 totalBONQRewardsLeft = _amountForRewards;297 emit TotalBONQRewardsUpdated(totalBONQRewardsLeft);298}299
300function setRouter(address _router) public onlyOwner {301 router = IRouter(_router);302 stableCoin.approve(_router, MAX_INT);303}Recommendation:
We advise the functions to be omitted or revised as they currently pose a significant centralization risk to the protocol, especially in the latter case in which all stablecoin funds are approved to the arbitrary address that is provided.
Alleviation:
The Bonq Protocol team has partially fixed this issue by removing the setBONQToken function while the setRouter is still present in the contract.
STA-02M: Inexistent Re-Entrancy Protection
| Type | Severity | Location |
|---|---|---|
| Logical Fault |  | stability-pool.sol:L604 |
Description:
It is possible to sabotage the contract by artificially inflating the P parameter via a re-entrancy attack on a malicious token added to the trading system of the router.
Impact:
It is currently possible to make a significant portion of funds in the contract unclaimable by exploiting a re-entrancy attack in the router's trade sequence to re-invoke the arbitrage function and measure the same delta in assets multiple times.
Example:
593function arbitrage(594 uint256 _amountIn,595 address[] calldata _path,596 uint256 _deadline597) public {598 require(_path[0] == address(stableCoin), "eafe8 must start with stable coin");599 require(_path[_path.length - 1] == address(stableCoin), "eafe8 must end with stable coin");600 // if the deadline was not set it is set to NOW - as the swap will happen in the same block it will be soon enough601 _deadline = _deadline > 0 ? _deadline : block.timestamp;602 uint256 startBalance = stableCoin.balanceOf(address(this));603 // the swap must yield at least 1 coin (in ETH parlance: 1 Wei) more than what was put in and the TX has 10 minutes to execute604 router.swapExactTokensForTokens(_amountIn, _amountIn + 1, _path, address(this), _deadline);605 uint256 amountOut = stableCoin.balanceOf(address(this)) - startBalance;606 // increase P by the arbitrage gain / total deposit607 _updateP((amountOut * DECIMAL_PRECISION) / totalDeposit, false);608 emit Arbitrage(_path, _amountIn, amountOut);609}Recommendation:
We advise a nonReentrant modifier to be introduced throughout the contract to disallow this kind of attack as otherwise the balance measurements utilized are unsafe.
Alleviation:
The Bonq Protocol team has added the nonReentrant modifier to the arbitrage function.