Lesson 15: Security Best Practices - Auditing Hooks

Vulnerable:

// VULNERABLE: Wrong comparison operator
int64_t amount = otxn_amount();
if (amount > MIN_AMOUNT) {  // Should be >=
    accept(SBUF("OK"), 0);
}
rollback(SBUF("Too small"), 1);

The > instead of >= means payments of exactly MIN_AMOUNT are rejected.

Fixed:

// FIXED: Correct comparison
int64_t amount = otxn_amount();
if (amount >= MIN_AMOUNT) {
    accept(SBUF("OK"), 0);
}
rollback(SBUF("Too small"), 1);

Vulnerable:

// VULNERABLE: Missing else/return
int64_t type = otxn_type();
if (type == TT_PAYMENT) {
    // Process payment
    accept(SBUF("Payment OK"), 0);
}
// Falls through to accept for ALL transaction types!
accept(SBUF("OK"), 0);

Fixed:

// FIXED: Explicit handling
int64_t type = otxn_type();
if (type == TT_PAYMENT) {
    // Process payment
    accept(SBUF("Payment OK"), 0);
} else {
    // Explicitly decide: accept or reject non-payments
    accept(SBUF("Non-payment OK"), 0);
}
// Unreachable - both branches call accept

Vulnerable:

// VULNERABLE: Inverted whitelist check
int whitelisted = 0;
for (int i = 0; GUARD(3), i < 3; ++i) {
    if (/* check match */) {
        whitelisted = 1;
        break;
    }
}

if (whitelisted) { // WRONG: Should be !whitelisted to reject
rollback(SBUF("Not whitelisted"), 1);
}
accept(SBUF("OK"), 0);
```

Fixed:

// FIXED: Correct logic
if (!whitelisted) {
    rollback(SBUF("Not whitelisted"), 1);
}
accept(SBUF("OK"), 0);

---

Vulnerable:

// VULNERABLE: Overflow not checked
int64_t total = 0;
for (int i = 0; GUARD(10), i < 10; ++i) {
    total += amounts[i];  // Could overflow
}

Fixed:

// FIXED: Check for overflow
int64_t total = 0;
for (int i = 0; GUARD(10), i < 10; ++i) {
    if (amounts[i] > INT64_MAX - total) {
        rollback(SBUF("Overflow detected"), 1);
    }
    total += amounts[i];
}

Vulnerable:

// VULNERABLE: Order of operations loses precision
int64_t share = (amount / 100) * percent;
// If amount=99 and percent=50, share=0 (99/100=0)

Fixed:

// FIXED: Multiply before divide
int64_t share = (amount * percent) / 100;
// If amount=99 and percent=50, share=49
// Still check for overflow on multiplication

Vulnerable:

// VULNERABLE: Doesn't handle IOU indicator
int64_t amount = otxn_amount();
if (amount < 1000000) {
    rollback(SBUF("Too small"), 1);
}
// IOU returns negative! -1 < 1000000 but it's not "too small"

Fixed:

// FIXED: Check for IOU first
int64_t amount = otxn_amount();
if (amount < 0) {
    rollback(SBUF("XRP only"), 1);
}
if (amount < 1000000) {
    rollback(SBUF("Too small"), 2);
}

---

Vulnerable:

// VULNERABLE: No return value check
uint8_t dest[20];
otxn_field(SBUF(dest), sfDestination);
// Using dest without verifying it was filled

Fixed:

// FIXED: Always check returns
uint8_t dest[20];
int64_t len = otxn_field(SBUF(dest), sfDestination);
if (len != 20) {
    rollback(SBUF("Invalid destination"), 1);
}

Vulnerable:

// VULNERABLE: Buffer too small
uint8_t hash[20];  // Hashes are 32 bytes!
otxn_id(SBUF(hash), 0);  // Writes 32 bytes to 20-byte buffer

Fixed:

// FIXED: Correct buffer size
uint8_t hash[32];
int64_t len = otxn_id(SBUF(hash), 0);
if (len != 32) {
    rollback(SBUF("Failed to get txid"), 1);
}

Vulnerable:

// VULNERABLE: Using parameter as address directly
uint8_t recipient[20];
otxn_param(SBUF(recipient), SBUF("to"));
// No validation! Could be any address

PREPARE_PAYMENT_SIMPLE(tx, amount, recipient, 0, 0);
emit(SBUF(hash), SBUF(tx));
```

Fixed:

// FIXED: Validate parameter
uint8_t recipient[20];
int64_t len = otxn_param(SBUF(recipient), SBUF("to"));

if (len != 20) {
rollback(SBUF("Invalid recipient param"), 1);
}

// Optionally: Check against whitelist
if (!is_approved_recipient(recipient)) {
rollback(SBUF("Recipient not approved"), 2);
}

PREPARE_PAYMENT_SIMPLE(tx, amount, recipient, 0, 0);
emit(SBUF(hash), SBUF(tx));
```

---

Vulnerable:

// VULNERABLE: Uninitialized state used as-is
uint8_t counter_buf[8];
state(SBUF(counter_buf), SBUF("counter"));
// If state doesn't exist, counter_buf contains garbage!
int64_t counter = INT64_FROM_BUF(counter_buf);

Fixed:

// FIXED: Check state existence
uint8_t counter_buf[8];
int64_t len = state(SBUF(counter_buf), SBUF("counter"));
int64_t counter;
if (len == 8) {
    counter = INT64_FROM_BUF(counter_buf);
} else {
    counter = 0;  // Initialize
}

Vulnerable:

// VULNERABLE: State can be modified by any transaction to this account
// If multiple Hooks or external access can modify state,
// assumptions may be violated

uint8_t admin[20];
state(SBUF(admin), SBUF("admin"));
// What if someone replaced admin?
```

Fixed:

// FIXED: Validate state integrity or use immutable configuration
// Option 1: Verify state matches expected format
uint8_t admin[20];
int64_t len = state(SBUF(admin), SBUF("admin"));
if (len != 20) {
    rollback(SBUF("Admin not configured"), 1);
}

// Option 2: Use Hook parameters (set at install time, immutable)
uint8_t admin[20];
int64_t len = hook_param(SBUF(admin), SBUF("admin"));
```

Hooks execute atomically per transaction, but state can change between transactions:

Consideration:

// Transaction 1: Read balance = 100
// Transaction 2: Read balance = 100 (same ledger)
// Both might emit based on "100" but only one will succeed

// Mitigation: Use state to track pending operations
// Or: Design to be idempotent
```

---

Vulnerable:

// VULNERABLE: No limit on emissions
int64_t amount = otxn_amount();
int count = amount / 1000000;  // 1 emission per XRP

etxn_reserve(count); // Could be huge!

for (int i = 0; GUARD(1000), i < count; ++i) {
// Emit many transactions
}
```

Fixed:

// FIXED: Limit emissions
#define MAX_EMISSIONS 5

int64_t amount = otxn_amount();
int count = amount / 1000000;

if (count > MAX_EMISSIONS) {
count = MAX_EMISSIONS;
}

etxn_reserve(count);
// ...
```

Vulnerable:

// VULNERABLE: Emission fees drain Hook account
// If triggered frequently with many emissions,
// account balance depletes

etxn_reserve(4);
// 4 emissions × ~12 drops each = ~48 drops per trigger
// 1000 triggers = 48,000 drops depleted
```

Fixed:

// FIXED: Check balance before emitting
int64_t balance = get_available_balance();
int64_t est_fees = num_emissions * 50;  // Conservative estimate

if (balance < est_fees + 10000000) { // Keep 10 XRP buffer
rollback(SBUF("Low balance, emissions paused"), 1);
}
```

Vulnerable:

// VULNERABLE: Emit to sender without validation
uint8_t sender[20];
otxn_field(SBUF(sender), sfAccount);

// Auto-refund logic
PREPARE_PAYMENT_SIMPLE(tx, refund_amount, sender, 0, 0);
emit(SBUF(hash), SBUF(tx));

// Attacker sends 1 drop, triggers refund of more
```

Fixed:

// FIXED: Validate refund logic
if (refund_amount > received_amount) {
    rollback(SBUF("Invalid refund calculation"), 1);
}

if (refund_amount < 1000) { // Minimum to cover fees
// Don't emit tiny refunds
accept(SBUF("Amount too small to refund"), 0);
}
```

---

SECURITY AUDIT CHECKLIST

INPUT VALIDATION:
□ All otxn_field returns checked
□ All otxn_param returns checked
□ All state returns checked
□ Buffer sizes match expected data
□ Integer bounds validated

LOGIC:
□ All branches explicitly handled
□ Correct comparison operators (< vs <=)
□ No inverted conditions
□ Default case handled in switches
□ Early returns don't skip cleanup

INTEGER SAFETY:
□ Overflow checked before arithmetic
□ Division by zero prevented
□ Multiplication before division
□ Negative values handled
□ Cast safety verified

STATE:
□ Initialization handled
□ State format validated
□ Updates are atomic
□ Cleanup of old state

EMISSIONS:
□ Emission count limited
□ Fee budget checked
□ Recipients validated
□ Amounts verified
□ Callback handles failures

GUARDS:
□ All loops have guards
□ Guard values adequate
□ Nested guard multiplication correct
□ No guard undercount
```

SECURITY TEST CASES

BOUNDARY TESTS:
□ Minimum values
□ Maximum values
□ Zero values
□ Negative values (IOUs)
□ Just above/below thresholds

ERROR PATHS:
□ Missing required fields
□ Invalid field formats
□ State not initialized
□ Insufficient balance
□ Failed emissions

ATTACK SCENARIOS:
□ Malicious sender
□ Invalid parameters
□ State manipulation attempts
□ Rapid repeated triggers
□ Complex transaction types

INTEGRATION:
□ Works with other Hooks
□ Handles emission callbacks
□ State consistency across transactions
```

---

// Layer 1: Type check
if (otxn_type() != TT_PAYMENT) {
    accept(SBUF("Not payment"), 0);
}

// Layer 2: Amount check
int64_t amount = otxn_amount();
if (amount < 0) {
rollback(SBUF("XRP only"), 1);
}
if (amount < MIN_AMOUNT) {
rollback(SBUF("Too small"), 2);
}

// Layer 3: Sender check
uint8_t sender[20];
if (otxn_field(SBUF(sender), sfAccount) != 20) {
rollback(SBUF("Invalid sender"), 3);
}

// Layer 4: Business logic check
if (!meets_business_rules(sender, amount)) {
rollback(SBUF("Business rules failed"), 4);
}

// Only now proceed with action
```

// Default to REJECT, explicitly ACCEPT
// Don't: accept at end, reject in conditions
// Do: reject at end, accept in conditions

if (valid_payment()) {
accept(SBUF("Valid"), 0);
}
if (whitelisted_sender()) {
accept(SBUF("Whitelisted"), 0);
}
// Default: reject unknown conditions
rollback(SBUF("Not explicitly allowed"), 99);
```

// Log critical decisions for post-incident analysis
trace(SBUF("Hook version: 1.2.3"), 0);
trace(SBUF("Sender:"), sender[0]);
trace(SBUF("Amount:"), amount);
trace(SBUF("Decision:"), decision_code);

// In production, balance verbosity with performance
```

---

✅ Most traditional smart contract attacks don't apply. Re-entrancy, flash loans, etc. are structurally impossible.

✅ Logic errors are the primary risk. Incorrect conditions, unchecked returns, integer issues.

✅ Guards prevent infinite loops. Execution is always bounded.

⚠️ New attack vectors may emerge. As ecosystem grows, new patterns may be discovered.

⚠️ Cross-Hook interactions. Multiple Hooks on same account could interact unexpectedly.

🔴 Assuming safety without auditing. The constrained model helps but doesn't eliminate all risk.

🔴 Unchecked return values. The most common vulnerability in practice.

🔴 Integer arithmetic errors. Overflow and truncation can silently corrupt calculations.

Hooks are inherently more secure than general smart contracts due to their constrained model. But logic errors, input validation failures, and integer bugs remain possible. Security requires disciplined coding practices, comprehensive testing, and careful code review. Use the checklists.

---

Assignment: Conduct a security audit of a Hook.

Requirements:

Option A: Audit the Payment Splitter from Lesson 14
Option B: Audit your own project from Lesson 14
Option C: Audit the provided vulnerable Hook (below)

Vulnerable Hook for Option C:

#include "hookapi.h"

int64_t hook(uint32_t reserved) {
    int64_t amount = otxn_amount();

uint8_t recipient[20];
    otxn_param(SBUF(recipient), SBUF("to"));

uint8_t count_buf[8];
    state(SBUF(count_buf), SBUF("count"));
    int64_t count = INT64_FROM_BUF(count_buf);

count = count + 1;
    INT64_TO_BUF(count_buf, count);
    state_set(SBUF(count_buf), SBUF("count"));

if (amount > 100000000) {
        etxn_reserve(1);
        uint8_t tx[PREPARE_PAYMENT_SIMPLE_SIZE];
        PREPARE_PAYMENT_SIMPLE(tx, amount / 10, recipient, 0, 0);
        uint8_t hash[32];
        emit(SBUF(hash), SBUF(tx));
    }

accept(SBUF("OK"), 0);
    return 0;
}

int64_t cbak(uint32_t r) { return 0; }

Audit Report Must Include:

1. Executive Summary (10%)

2. Vulnerability List (40%)

3. Fixed Code (30%)

4. Test Cases (20%)

• Completeness of vulnerability identification (40%)
• Quality of fix recommendations (30%)
• Fixed code correctness (20%)
• Report clarity (10%)

Time investment: 4-6 hours
Value: Critical skill for production Hook development

---

For Next Lesson:
We'll cover performance optimization—making Hooks faster and more efficient.

---

End of Lesson 15

Total words: ~4,400
Estimated completion time: 60 minutes reading + 4-6 hours for deliverable