Most sensitive scope on Stability Platform v24.01.1-alpha version. The audit was conducted as part of a closed contest among students of technical universities and security enthusiasts.
Manual review
71ac9d219d8dde8b88b45f31d4edf5ea91abd27d
The smart contract code review of the solidity blockchain code was performed in such way in order to identify any low to high severity discrepancies within the smart contracts themselves, which may be used as indication to measure the extend of the damages that a black hat hacker could achieve just exploiting the solidity bugs within the assets in scope.
| File Path | nSLOC |
|---|---|
| src/core/base/VaultBase.sol | 572 |
| src/core/vaults/CVault.sol | 58 |
After having performed and gone through several key stages in the smart contract code review, we were able to identify 3 (three) total solidity bugs/vulnerability, however none were highly compelling and do not require emergency patches.
Overall, Stability was found to have robust smart contracts (from the ones in scope), with some minor solidity artifacts and best practice changes.
This section covers the primary vulnerabilities that have been confirmed, through testing, to be present in the target(s) in scope, and would thereby likely allow unwanted, unintended or unauthorised actions on through the smart contract. Each key vulnerability is given an overall risk rating based off its individual impact on the target. As part of the sample limitations, remediation is not always included and the description is less detailed.
| Severity | Count |
|---|---|
| High | 0 |
| Medium | 2 |
| Low | 2 |
| Aspect | Details |
|---|---|
| Rating | Medium |
| Impact | Withdrawals blocking by griefing attack |
| Aspect | Details |
|---|---|
| Rating | Medium |
| Impact | The attacker can obtain income belonging to other vault holders |
| Aspect | Details |
|---|---|
| Rating | Low |
| Impact | Separate ISwapper getPrice interfaces |
| Aspect | Details |
|---|---|
| Rating | Low |
| Impact | The user may receive less commission when exiting the strategy before HardWork |
stability-contracts/src/core/base/VaultBase.sol
Lines 568 to 573 in 5a22ba1
Anyone can block the withdrawal possibility of any user. The attacker can block users with big deposits for a long time just transferring them 0 amount of shares before the withdrawal delay expires. The victim will not be able to withdraw assets instantly. This can cause different financial losses depending on the withdrawal purpose. The time of blocking will be controlled by the attacker, because the delay parameter in the contract is a constant and can't be set to 0 to interrupt an attack. There is a similar issue https://solodit.xyz/issues/m-10-griefing-attack-to-block-withdraws-code4rena-mochi-mochi-contest-git.
The contract establish a delay between deposits/transfers and withdrawals:
uint internal constant _WITHDRAW_REQUEST_BLOCKS = 5;Users can't withdraw assets before the delay:
function _beforeWithdraw(VaultBaseStorage storage $, address owner) internal {
if ($.withdrawRequests[owner] + _WITHDRAW_REQUEST_BLOCKS >= block.number) {
revert WaitAFewBlocks();
}
$.withdrawRequests[owner] = block.number;
}Every time when user receives shares the user's withdrawRequests is set to block.number:
function _update(address from, address to, uint value) internal
virtual override {
super._update(from, to, value);
VaultBaseStorage storage $ = _getVaultBaseStorage();
$.withdrawRequests[from] = block.number;
$.withdrawRequests[to] = block.number;
}It will happen even for a zero value transfer.
The only way for the victim to try to withdraw is to split the balance between many addresses. This can somehow make the attack more difficult.
Still on it
If an attacker adds funds to the vault before the HardWork become and withdraw them immediately, he receives the income that belong to the remaining holders of the vault.
Use delayed HardWork rewarding in form of a vesting (like RVault). This can increase gas consumption.
All contracts that implement ISwapper (UniswapV3Adapter, AlgebraAdapter) use
(uint160 sqrtPriceX96,,,,,,) = IAlgebraPool(pool).globalState();to obtain the price that is used to calculate tvl.
The sqrtPriceX96 variable can be manipulated to increase or decrease the tvl of the protocol using flash-loan.
To avoid such situations, you should separate the use of ISwapper.getPrice. Use one function for exchange and leave it as is, the second for displaying tvl, etc. and use Oracle.Observation from Uniswap V3.
Strategy.doHardWork collects revenue
Strategy.doHardWork can be called either from VaultBase.doHardWork or from VaultBase.depositAssets.
VaultBase can be created with any strategy that has doHardWork on deposit option enabled
The following scenario is possible:
- The user adds liquidity to the vault through depositAssets;
- In a strategy, yield are accrued;
- The user calls
withdrawAssets; - As a result, if there was no call to
doHardWorkbetween steps 1 and 3, then the pool fee rewards will not be credited to the user.
Add doHardWorkOnWithdraw option to VaultBase, Add a doHardWork call to beginning of withdrawAssets.