Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 202 additions & 0 deletions ANALYTICS_AUTH_FIX.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
# Analytics Contract Authentication Vulnerability Fix

## Issue Summary

The analytics contract's `record_metric` and `record_metric_at` functions had no authentication requirement. This allowed any account to write arbitrary data to metric streams, creating the following risks:

- **Data Integrity**: Attackers could flood streams with false data
- **Capacity Exhaustion**: Metric storage could be depleted by attackers
- **Leaderboard Manipulation**: Analytics dashboards relying on metric data could be corrupted
- **System Compromise**: Invalid metrics could corrupt upstream systems depending on analytics

## Solution Implemented

### 1. Authorized Reporter Pattern

The fix implements a **whitelist-based authorization model** where only registered "authorized reporters" can record metrics.

```
Admin ──[manages]──> Authorized Reporters ──[authorized to record]──> Metrics
```

### 2. Key Functions

#### Initialization
- **`initialize(admin: Address)`** - One-time setup, stores admin address and initializes empty reporter list
- ✅ Called once per contract deployment
- ❌ Subsequent calls fail with `AlreadyInitialized`

#### Admin Management (Admin-Only)
- **`add_authorized_reporter(reporter: Address)`** - Add to whitelist
- Requires admin authentication via `require_auth()`
- Prevents duplicate entries
- Returns `Unauthorized` if caller is not admin

- **`remove_authorized_reporter(reporter: Address)`** - Remove from whitelist
- Requires admin authentication via `require_auth()`
- Returns `MarketNotFound` if reporter not in list

#### Read-Only Queries
- **`is_authorized_reporter(reporter: Address) -> bool`** - Check reporter status
- No authentication required
- Read-only, no state changes

- **`get_admin() -> Address`** - Retrieve admin address
- No authentication required
- Read-only, no state changes

#### Metric Recording (Reporter-Only)
- **`record_metric(metric_name: Symbol, value: u64) -> MetricData`**
- Uses current ledger timestamp
- Requires caller to be in authorized reporter whitelist
- Calls `require_auth()` for transaction signing
- Returns `Unauthorized` if caller not authorized
- Returns `NotInitialized` if contract not set up

- **`record_metric_at(metric_name: Symbol, timestamp: u64, value: u64) -> MetricData`**
- Uses explicit timestamp
- Same authorization requirements as `record_metric`
- Prevents duplicate entries (same metric_name + timestamp)
- Returns `DuplicateEntry` if already recorded

### 3. Error Codes

New and existing error codes used:
- `1` - `Unauthorized` - Caller lacks required role
- `2` - `AdminNotSet` - Admin not initialized
- `3` - `NotInitialized` - Contract not initialized
- `4` - `AlreadyInitialized` - Already initialized
- `10` - `MarketNotFound` - Reporter not in whitelist (reused for removal check)
- `22` - `DuplicateEntry` - Duplicate metric entry

## Auth Boundary

### Protected Operations
All state-changing operations are protected:
```
initialize(admin) → Auth: First call only
add_authorized_reporter(reporter) → Auth: Admin required
remove_authorized_reporter(reporter) → Auth: Admin required
record_metric(name, value) → Auth: Authorized reporter required
record_metric_at(name, timestamp, val) → Auth: Authorized reporter required
```

### Public Read-Only Operations
No authentication required:
```
get_admin() → Returns Address
is_authorized_reporter(addr) → Returns bool
aggregate(event_types) → Returns u64
percentile(values, pct) → Returns u64
```

## Test Coverage

The `tests/auth_boundary.rs` file provides 24 comprehensive tests covering:

### Initialization (2 tests)
- ✓ First initialization succeeds
- ✓ Duplicate initialization fails with AlreadyInitialized

### Reporter Management (7 tests)
- ✓ Only admin can add reporters
- ✓ Adding reporter succeeds when admin
- ✓ Duplicate reporters prevented
- ✓ Only admin can remove reporters
- ✓ Removing reporter succeeds when admin
- ✓ Removing non-existent reporter fails
- ✓ Reporter authorization query works

### Metric Recording (10 tests)
- ✓ Unauthorized caller rejected from record_metric
- ✓ Authorized reporter can record_metric
- ✓ Unauthorized caller rejected from record_metric_at
- ✓ Authorized reporter can record_metric_at
- ✓ Duplicate metric entries prevented
- ✓ Metrics require initialization
- ✓ Revoked reporters cannot record after removal
- ✓ get_admin works without auth
- ✓ is_authorized_reporter works without auth
- ✓ aggregate works without auth

### Read-Only Operations (3 tests)
- ✓ aggregate and percentile work without authentication
- ✓ get_admin and is_authorized_reporter callable by anyone
- ✓ Read-only operations don't modify state

## Integration

### For Metrics Producers (e.g., Markets Contract)

1. **Setup (Admin)**
```rust
// Initialize once
analytics.initialize(&admin_address);

// Add markets contract as authorized reporter
analytics.add_authorized_reporter(&markets_contract_address);
```

2. **Recording Metrics (Markets Contract)**
```rust
// Record metrics from price feed
analytics.record_metric(
Symbol::new(&env, "markets:price"),
current_price
)?;

// Record with historical timestamp
analytics.record_metric_at(
Symbol::new(&env, "markets:resolved_event"),
resolution_time,
final_outcome
)?;
```

### For Analytics Consumers
```rust
// Read metrics (no auth needed)
let is_authorized = analytics.is_authorized_reporter(&some_address)?;
let admin = analytics.get_admin()?;
let avg = analytics.aggregate(&event_types)?;
let median = analytics.percentile(&values, &50)?;
```

## Security Properties

✅ **Principle of Least Privilege**: Only authorized reporters can write metrics
✅ **Admin Centralization**: Single admin controls reporter list
✅ **Immutable Audit Trail**: Metrics cannot be modified after recording
✅ **Duplicate Prevention**: Same metric cannot be recorded twice
✅ **Authentication Enforcement**: Soroban `require_auth()` enforces transaction signing
✅ **Read-Only Safety**: Query operations require no auth and make no state changes

## Migration Notes

This is a **new implementation** of `record_metric` and `record_metric_at` functions. If previous implementations exist:

1. Deploy new contract with this code
2. Initialize with admin address
3. Add authorized reporters
4. Re-record metrics from production systems

The old implementation should be deprecated and replaced with calls to the new authenticated functions.

## Files Modified

- `contracts/analytics/src/lib.rs` - Main contract implementation
- `contracts/analytics/tests/auth_boundary.rs` - Comprehensive test suite
- `contracts/analytics/src/errors.rs` - Unchanged (already has required error codes)

## Build & Test

```bash
# Run auth boundary tests
cargo test --test auth_boundary -- --nocapture

# Run all tests
cargo test --workspace

# Build contract
cargo build --target wasm32-unknown-unknown --release
```
Loading
Loading