add vuln, lint

stabilitydao · Apr 1, 2024 · 9a5e969 · 9a5e969
1 parent 2714b86
commit 9a5e969
Showing 1 changed file with 62 additions and 43 deletions.
diff --git a/audit/initial-audit-stability-platform-v24.01.1-alpha.md b/audit/initial-audit-stability-platform-v24.01.1-alpha.md
@@ -1,7 +1,10 @@
 # Stability initial Vault audit (11.03.24)
-Most sensitive scope on Stability Platform v24.01.1-alpha stage
+
+Most sensitive scope on Stability Platform v24.01.1-alpha version.
+
 ## Audited Commit
-https://github.com/stabilitydao/stability-contracts/commit/71ac9d219d8dde8b88b45f31d4edf5ea91abd27d
+
+[71ac9d219d8dde8b88b45f31d4edf5ea91abd27d](https://github.com/stabilitydao/stability-contracts/commit/71ac9d219d8dde8b88b45f31d4edf5ea91abd27d)
 
 ## Executive Summary
 
@@ -21,6 +24,7 @@ After having performed and gone through several key stages in the smart contract
 Overall, Stability was found to have robust smart contracts (from the ones in scope), with some minor solidity artifacts and best practice changes.
 
 ## Vulnerability Detail
+
 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.
@@ -29,46 +33,55 @@ risk rating based off its individual impact on the target. As part of the sample
 |----------|-------|
 | High     | 0     |
 | Medium   | 2     |
-| Low      | 1     |
+| Low      | 2     |
+
+### M-01
 
-## M-01
 | Aspect          | Details                                                                                                 |
 |-----------------|---------------------------------------------------------------------------------------------------------|
 | Rating          | Medium                                                                                                     |
 | Impact          | Withdrawals blocking by griefing attack |
-## M-02
+
+### M-02
+
 | Aspect          | Details                                                                                                 |
 |-----------------|---------------------------------------------------------------------------------------------------------|
 | Rating          | Medium                                                                                                     |
-| Impact          | The user may receive less commission when exiting the strategy |
-## L-01
+| Impact          | The attacker can obtain income belonging to other vault holders |
+
+### L-01
+
 | Aspect          | Details                                                                                                 |
 |-----------------|---------------------------------------------------------------------------------------------------------|
-| Rating          | Low |                                                                                      
+| Rating          | Low |
 | Impact          | Separate ISwapper getPrice interfaces |
 
+### L-02
+
+| Aspect          | Details                                                                        |
+|-----------------|--------------------------------------------------------------------------------|
+| Rating          | Low                                                                         |
+| Impact          | The user may receive less commission when exiting the strategy before HardWork |
 
 # Medium Risk
 
 ## M-01 Withdrawals blocking by griefing attack
 
-## Links to affected code
+### Links to affected code
 
 https://github.com/stabilitydao/stability-contracts/blob/5a22ba12dbd2ddae0b1d68706f2697b64e3012a9/src/core/base/VaultBase.sol#L35
 
 https://github.com/stabilitydao/stability-contracts/blob/5a22ba12dbd2ddae0b1d68706f2697b64e3012a9/src/core/base/VaultBase.sol#L568-L573
 
 https://github.com/stabilitydao/stability-contracts/blob/5a22ba12dbd2ddae0b1d68706f2697b64e3012a9/src/core/base/VaultBase.sol#L512
 
-## Vulnerability details
+### Vulnerability details
 
-## Impact
+### Impact
 
 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.
 
-
-
-## Proof of Concept
+### Proof of Concept
 
 The contract establish a delay between deposits/transfers and withdrawals:
 
@@ -103,59 +116,65 @@ 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.
 
-
-## Recommended Mitigation Steps
+### Recommended Mitigation Steps
 
 Still on it
 
+## M-02 The attacker can obtain income belonging to other vault holders
 
-## M-02 The user may receive less commission when exiting the strategy
-
-## Vulnerability details
-
-## Impact
-`Strategy.doHardWork` collects revenue from the DAO.
+### Vulnerability details
 
-`Strategy.doHardWork` can be called either from `VaultBase.doHardWork` or from `VaultBase.depositAssets`.
-
-`VaultBase` can be created with the `QuickSwapV3StaticFarmStrategy` — this strategy collects fees from uni-v3.
-
-The following scenario is possible:
+### Impact
 
-1.  The user adds liquidity to the vault through depositAssets;
-2.  In `QuickSwapV3StaticFarmStrategy`, trading occurs and fees are accrued;
-3.  The user calls `withdrawAssets`;
-4.  As a result, if there was no call to `doHardWork` between steps 1 and 3, then the pool fee rewards will not be credited to the user.
+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.
 
+### Recommended Mitigation Steps
 
+Use delayed HardWork rewarding in form of a vesting (like `RVault`). This can increase gas consumption.
 
-## Recommended Mitigation Steps
+## L-01 Separate ISwapper getPrice interfaces
 
-Add a doHardWork call to beginning of withdrawAssets.
+### Vulnerability details
 
-## L-01 Separate ISwapper getPrice interfaces
-## Vulnerability details
+### Impact
 
-## Impact
 All contracts that implement ISwapper (UniswapV3Adapter, AlgebraAdapter) use
 
-```solidity 
+```solidity
 (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.
 
-
-## Recommended Mitigation Steps
+### Recommended Mitigation Steps
 
 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.
-
 
-## Tools Used
+## L-02 The user may receive less income when exiting the vault before HardWork
 
-Manual review
+### Vulnerability details
+
+### Impact
+
+`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:
+
+1. The user adds liquidity to the vault through depositAssets;
+2. In a strategy, yield are accrued;
+3. The user calls `withdrawAssets`;
+4. As a result, if there was no call to `doHardWork` between steps 1 and 3, then the pool fee rewards will not be credited to the user.
 
+### Recommended Mitigation Steps
 
+Add `doHardWorkOnWithdraw` option to `VaultBase`, Add a doHardWork call to beginning of withdrawAssets.
+
+## Tools Used
+
+Manual review