Skip to content

Commit 57d4094

Browse files
kupermindkupermind
authored
Merge pull request #13 from LemonTreeTechnologies/audit8
doc: internal audit8
2 parents 3fdac27 + 7be90f3 commit 57d4094

4 files changed

Lines changed: 701 additions & 1 deletion

File tree

‎audits/README.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ An `audit3` with a focus `post-internal-audit` branch [audit3](https://github.co
88
An `audit4` with a focus `post-lock` branch [audit4](https://github.com/kupermind/olas-lst/blob/main/audits/audit4). <br>
99
An `audit5` with a focus `main` branch [audit5](https://github.com/kupermind/olas-lst/blob/main/audits/audit5). <br>
1010
An `audit6` with a focus `main` branch [audit6](https://github.com/kupermind/olas-lst/blob/main/audits/audit6). <br>
11-
An `audit7` with a focus `main` branch [audit6](https://github.com/LemonTreeTechnologies/olas-lst/blob/main/audits/audit7). <br>
11+
An `audit7` with a focus `main` branch [audit7](https://github.com/LemonTreeTechnologies/olas-lst/blob/main/audits/audit7). <br>
12+
An `audit8` with a focus `main` branch [audit8](https://github.com/LemonTreeTechnologies/olas-lst/blob/main/audits/audit8). <br>
13+
An `audit9` — full project re-audit + delta review [audit9](https://github.com/LemonTreeTechnologies/olas-lst/blob/main/audits/audit9). <br>
1214

1315
### External audits
1416
External audit reports are listed in their historical order:

‎audits/audit8/README.md‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
# Security Audit Report — olas-lst (stOLAS)
2+
3+
**Date**: 2026-03-08
4+
**Scope**: All contracts in `contracts/` (excluding `lib/`, `test/`, `contracts/l1/concept/`, `contracts/test/`)
5+
**Lines of Code**: 8,086 LOC across 29 contracts
6+
7+
---
8+
9+
## Executive Summary
10+
11+
The stOLAS liquid staking protocol was audited following the 5-phase security audit methodology. The codebase implements a cross-chain ERC4626 vault system with L1 (Ethereum) deposits/withdrawals and L2 (Gnosis/Base) staking operations.
12+
13+
**Result: No bounty-qualifying vulnerabilities found.**
14+
15+
The protocol demonstrates mature defensive coding patterns:
16+
- Internal accounting model (totalReserves) immune to donation-based price manipulation
17+
- Time-delayed withdrawals preventing flash loan and sandwich attacks
18+
- Per-contract reentrancy guards (transient storage on L1, persistent on L2)
19+
- Strict access control with one-caller-per-function pattern on stOLAS
20+
- Optimistic cross-chain accounting with recovery mechanisms for failed bridge messages
21+
22+
4 informational observations were documented. The protocol has undergone 7 internal and 1 external audit (CODESPECT), and the code quality reflects this maturity.
23+
24+
---
25+
26+
## Scope
27+
28+
### In-Scope Contracts
29+
30+
**L1 (Ethereum) — 2,633 LOC**
31+
| Contract | LOC | Description |
32+
|----------|-----|-------------|
33+
| stOLAS.sol | 378 | ERC4626 vault, main entry point |
34+
| Depository.sol | 1,139 | Staking model management, cross-chain bridge hub |
35+
| Treasury.sol | 303 | Withdrawal queue, ERC6909 claim tokens |
36+
| Distributor.sol | 148 | Reward split (veOLAS lock + vault) |
37+
| Lock.sol | 287 | veOLAS lock management, governance proxy |
38+
| UnstakeRelayer.sol | 80 | Retired model unstake relay |
39+
40+
**L1 Bridging — 738 LOC**
41+
| Contract | LOC | Description |
42+
|----------|-----|-------------|
43+
| DefaultDepositProcessorL1.sol | 182 | Abstract bridge base |
44+
| BaseDepositProcessorL1.sol | 116 | Optimism/Base bridge |
45+
| GnosisDepositProcessorL1.sol | 62 | Gnosis AMB bridge |
46+
| LzOracle.sol | 378 | LayerZero lzRead oracle (NOT currently used) |
47+
48+
**L2 (Gnosis/Base) — 3,851 LOC**
49+
| Contract | LOC | Description |
50+
|----------|-----|-------------|
51+
| StakingManager.sol | 556 | Service orchestration |
52+
| ExternalStakingDistributor.sol | 1,123 | External staking management |
53+
| StakingTokenLocked.sol | 848 | Locked staking instance |
54+
| ActivityModule.sol | 342 | Service activity + claim |
55+
| Collector.sol | 406 | Reward collection + bridging |
56+
| StakingHelper.sol | 77 | View helper |
57+
| MultisigGuard.sol | 216 | Safe guard for externals |
58+
| ModuleActivityChecker.sol | 33 | Activity nonce checker |
59+
60+
**L2 Bridging — 679 LOC**
61+
| Contract | LOC | Description |
62+
|----------|-----|-------------|
63+
| DefaultStakingProcessorL2.sol | 473 | Abstract bridge base |
64+
| BaseStakingProcessorL2.sol | 113 | Optimism/Base bridge |
65+
| GnosisStakingProcessorL2.sol | 93 | Gnosis bridge |
66+
67+
**Infrastructure — 255 LOC**
68+
| Contract | LOC | Description |
69+
|----------|-----|-------------|
70+
| Beacon.sol | 62 | Beacon pattern |
71+
| BeaconProxy.sol | 60 | EIP-1967 beacon proxy |
72+
| Proxy.sol | 73 | UUPS-style proxy |
73+
| Implementation.sol | 60 | Base implementation with owner |
74+
75+
### Out of Scope
76+
- `lib/` dependencies (autonolas-registries, LayerZero, OpenZeppelin, solmate)
77+
- `contracts/test/` mock contracts
78+
- `contracts/l1/concept/` experimental contracts
79+
- Owner/multisig actions (trusted role per CLAUDE.md)
80+
- LzOracle.sol (confirmed not currently used)
81+
82+
---
83+
84+
## Findings
85+
86+
### Critical: 0
87+
### High: 0
88+
### Medium: 0
89+
### Low: 0
90+
91+
### Informational: 4
92+
93+
#### INFO-1: Missing ETH Forwarding in Treasury.requestToWithdraw
94+
| | |
95+
|---|---|
96+
| **Location** | Treasury.sol:220-221, 227-228 |
97+
| **Description** | `requestToWithdraw` is `payable` but doesn't forward `msg.value` to `Depository.unstake`/`unstakeExternal` calls |
98+
| **Impact** | None currently — Optimism and Gnosis bridges don't require ETH for L1→L2 messages. If a bridge requiring ETH is added, unstaking from Treasury would fail. |
99+
| **Recommendation** | Add `{value: ...}` forwarding or document that future bridges must not require ETH for L1→L2 UNSTAKE messages |
100+
101+
#### INFO-2: Dangling OLAS Approval in Distributor
102+
| | |
103+
|---|---|
104+
| **Location** | Distributor.sol:75 |
105+
| **Description** | If `Lock.increaseLock()` fails via low-level call, OLAS approval to Lock remains until next `distribute()` call |
106+
| **Impact** | None — Lock is immutable trusted address. Approval overwritten on next distribute(). |
107+
| **Recommendation** | Consider resetting approval to 0 after failed lock call |
108+
109+
#### INFO-3: ERC6909 Withdrawal Tokens Are Transferable
110+
| | |
111+
|---|---|
112+
| **Location** | Treasury.sol:190, solmate ERC6909 |
113+
| **Description** | Withdrawal claim tokens can be transferred. New holder can finalize withdrawal after delay. |
114+
| **Impact** | Feature, not bug. Enables secondary market for withdrawal claims. |
115+
| **Recommendation** | Document this behavior for users |
116+
117+
#### INFO-4: Multiple Permissionless Trigger Functions
118+
| | |
119+
|---|---|
120+
| **Location** | Depository.sol:763,829; Distributor.sol:125; UnstakeRelayer.sol:60; Collector.sol:317; ActivityModule.sol:303 |
121+
| **Description** | These functions (unstakeRetired, closeRetiredStakingModels, distribute, relay, relayTokens, claim) can be called by anyone. |
122+
| **Impact** | None — all are designed as permissionless triggers. Cannot cause fund loss when called by untrusted parties. |
123+
| **Recommendation** | N/A — by design |
124+
125+
---
126+
127+
## Methodology
128+
129+
### Phase 1: Architecture Map
130+
- Mapped 29 in-scope contracts across L1, L2, bridging, and infrastructure layers
131+
- Documented inheritance hierarchy, inter-contract call graph, access control matrix
132+
- Identified 10 priority attack surface areas
133+
134+
### Phase 2: Invariant Identification
135+
- Verified 10 core invariants (all holding)
136+
- Disproved 8 initial candidate findings through code verification
137+
- Identified 8 candidates for Phase 3 deep investigation
138+
139+
### Phase 3: Attack Discovery
140+
- Investigated all 8 Phase 2 candidates — all disproved
141+
- Ran 16-item EVM checklist — all SAFE
142+
- Ran 8-item DeFi attack pattern checklist — all SAFE
143+
- Applied disprove-first methodology to every finding
144+
145+
### Coverage
146+
147+
| Category | Items Checked |
148+
|----------|--------------|
149+
| Phase 2 candidates | 8/8 investigated |
150+
| EVM patterns | 16/16 checked |
151+
| DeFi patterns | 8/8 checked |
152+
| Invariants verified | 10/10 |
153+
| False positives disproved | 17 |
154+
| Informational findings | 4 |
155+
156+
---
157+
158+
## Key Architectural Observations
159+
160+
### Strengths
161+
1. **Internal accounting model**: `totalAssets()` returns `totalReserves` (not `balanceOf`), making the vault immune to donation-based attacks (ERC4626 inflation, share price manipulation)
162+
2. **Withdrawal delay**: Time-delayed withdrawals via Treasury prevent flash loans, sandwich attacks, and cross-chain arbitrage
163+
3. **Access control isolation**: Each stOLAS function has exactly one authorized caller — no cross-role attack vectors
164+
4. **Graceful degradation**: Distributor handles Lock failure gracefully (all rewards go to vault); L2 bridge queues failed operations for retry
165+
5. **Rounding direction**: Both deposit and redeem use `mulDivDown` (favor vault), preventing rounding extraction
166+
167+
### Design Trade-offs (Not Vulnerabilities)
168+
1. **Optimistic cross-chain accounting**: L1 updates balances before L2 confirms. Relies on operational liveness for recovery.
169+
2. **No virtual shares**: stOLAS doesn't implement OpenZeppelin's virtual shares offset. Mitigated by depository-only deposit access control.
170+
3. **ERC4626 non-standard functions**: `mint()` and `withdraw()` revert. May confuse integrators.
171+
172+
---
173+
174+
*Audit conducted using the Security Audit Playbook v2.8. All findings were verified using disprove-first methodology with exact file:line references.*

‎audits/audit8/external-staking-distributor-analysis.md‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
# Detailed Analysis: ExternalStakingDistributor.sol (1,123 LOC)
2+
3+
**Date**: 2026-03-08
4+
**Contract**: `contracts/l2/ExternalStakingDistributor.sol`
5+
6+
---
7+
8+
## Architecture Overview
9+
10+
The contract manages external staking services on L2 — creates Safe multisig wallets, stakes services in external staking proxies, handles reward distribution, and processes cross-chain unstake requests.
11+
12+
**Key roles:**
13+
- `owner` — configures proxies, agents
14+
- `l2StakingProcessor` — bridge endpoint (deposit/withdraw)
15+
- `managingAgents` — can trigger unstake
16+
- curating agents — can stake services (per-proxy access via `stakingGuard`)
17+
- anyone — can call `claim`, `unstakeAndWithdraw` if evicted/no rewards
18+
19+
---
20+
21+
## Detailed Findings
22+
23+
### 1. `reStake` uses dead `mapCuratingAgents` mapping (lines 804, 244-245)
24+
25+
**Line 244:** `// UNUSED for now: Mapping whitelisted curating agent addresses`
26+
**Line 245:** `mapping(address => bool) public mapCuratingAgents;`
27+
28+
**Line 804 (reStake):**
29+
```solidity
30+
if (!(mapCuratingAgents[msg.sender] || mapManagingAgents[msg.sender] || msg.sender == owner))
31+
```
32+
33+
**But `stake` (line 610) uses:**
34+
```solidity
35+
curatingAgentAccess = mapStakingGuardHashCuratingAgents[stakingHash][msg.sender];
36+
```
37+
38+
No function ever sets `mapCuratingAgents` to `true` — `setCuratingAgents` (line 912) writes to `mapStakingGuardHashCuratingAgents` instead. So the curating agent access check in `reStake` is dead code.
39+
40+
**Impact:** A curating agent who staked a service cannot restake it after eviction — only managing agents or owner can. No fund loss — managing agents/owner can still restake. Functionality limitation only.
41+
42+
**Severity:** Informational. The "UNUSED for now" comment suggests intentional deferral.
43+
44+
---
45+
46+
### 2. Reward distribution math — verified correct (lines 494-500)
47+
48+
```solidity
49+
collectorAmount = (reward * collectorAmount) / MAX_REWARD_FACTOR; // round DOWN
50+
protocolAmount = (reward * protocolAmount) / MAX_REWARD_FACTOR; // round DOWN
51+
uint256 fullCollectorAmount = collectorAmount + protocolAmount;
52+
uint256 curatingAgentAmount = reward - fullCollectorAmount; // remainder → curating agent
53+
```
54+
55+
Max dust loss: 2 wei per distribution (two round-downs). Curating agent absorbs remainder. Total always equals `reward`. **No issue.**
56+
57+
---
58+
59+
### 3. V1 DelegateCall to multiSend — verified safe (line 543)
60+
61+
```solidity
62+
ISafe(multisig).execTransactionFromModule(multiSend, 0, msPayload, ISafe.Operation.DelegateCall);
63+
```
64+
65+
Inner operations are all `ISafe.Operation.Call` (0x00). `multiSend` is immutable (line 220). MultiSendCallOnly rejects nested DelegateCall. **No issue.**
66+
67+
---
68+
69+
### 4. `unstakeAndWithdraw` permissionless path — verified safe (lines 697-702)
70+
71+
```solidity
72+
if (stakingState == IStaking.StakingState.Staked && availableRewards > 0
73+
&& !(mapManagingAgents[msg.sender] || msg.sender == owner)) {
74+
revert UnauthorizedAccount(msg.sender);
75+
}
76+
```
77+
78+
Anyone can unstake if evicted OR `availableRewards == 0`. Rewards go to collector/protocol/curatingAgent — caller gets nothing. **By design.**
79+
80+
---
81+
82+
### 5. `withdrawAndRequestUnstake` deficit accumulation — verified correct (lines 1005-1009)
83+
84+
```solidity
85+
if (amount > olasBalance) {
86+
unstakeRequestedAmount = amount - olasBalance;
87+
amount = olasBalance;
88+
mapUnstakeOperationRequestedAmounts[operation] += unstakeRequestedAmount;
89+
}
90+
```
91+
92+
Deficit accumulates per operation. Fulfilled in `unstakeAndWithdraw` (lines 741-770) with partial fulfillment support. **Correct accounting.**
93+
94+
---
95+
96+
### 6. OLAS approve patterns — verified
97+
98+
- V2 line 553: `approve(collector, fullCollectorAmount)` → consumed by `topUpBalance(collectorAmount)` + `topUpProtocol(protocolAmount)`. Exact match.
99+
- Line 764: `approve(collector, amount)` → consumed by `topUpBalance(amount)`. Exact match.
100+
- Line 659: `approve(serviceRegistryTokenUtility, fullStakingDeposit)`. Consumed during deploy+stake. Correct.
101+
102+
No dangling approvals in normal flow. **No issue.**
103+
104+
---
105+
106+
### 7. `stakedBalance` accounting — verified
107+
108+
| Function | Operation | Line |
109+
|----------|-----------|------|
110+
| `stake` | `+= fullStakingDeposit` | 656 |
111+
| `unstakeAndWithdraw` | `-= fullStakingDeposit` | 719 |
112+
| `reStake` | no change (same deposit) | — |
113+
114+
Symmetric. `reStake` correctly leaves `stakedBalance` unchanged since it unstakes and restakes the same deposit. **No issue.**
115+
116+
---
117+
118+
### 8. `_createMultisigWithSelfAsModule` — verified safe (lines 338-401)
119+
120+
- Creates Safe with `address(this)` as initial owner
121+
- Enables `address(this)` as module + guard as module
122+
- Sets guard
123+
- Swaps owner to `agentInstance`
124+
- Uses contract signature (`r = bytes32(uint256(uint160(address(this))))`, `v = 1`)
125+
126+
After execution: agentInstance is owner, ExternalStakingDistributor and guard are modules, guard is set. **Correct setup.**
127+
128+
---
129+
130+
## Summary
131+
132+
| Area | Status |
133+
|------|--------|
134+
| Reward math | Safe — remainder absorbs rounding |
135+
| DelegateCall | Safe — immutable target, Call-only inner ops |
136+
| Balance tracking | Safe — symmetric stake/unstake |
137+
| Access control | Functional — `reStake` curating agent check is dead code (informational) |
138+
| Cross-chain deficit | Safe — accumulates and partially fulfills |
139+
| Approve patterns | Safe — no dangling approvals |
140+
| Reentrancy | Protected — `_locked` guard on all state-changing functions |
141+
142+
**No new bounty-qualifying vulnerabilities found.** The `mapCuratingAgents` dead code in `reStake` is consistent with what we already noted as a design observation — it's marked "UNUSED for now" by the developers.

0 commit comments

Comments
 (0)