Skip to content

Incorrect maxAccountIndex() bound lets parseAccount() return phantom accounts from non-account slab data #370

Description

@Bayyan16
Severity High
Category Logic error — incorrect bounds computation leading to out-of-bounds read
Component src/solana/slab.ts
Affected @percolatorct/sdk v4.3.0 — verified at main = c976188
Status Confirmed by execution; fix applied, built and tested
CWE CWE-125 (Out-of-bounds Read), CWE-1284 (Improper Validation of Specified Quantity in Input)
Affects the v12.19 layout, i.e. the layout src/solana/slab.ts:970 records as deployed on mainnet

1. Summary

maxAccountIndex() derives the number of parseable account slots by dividing everything after accountsOff by accountSize. On the v12.19 layout that trailing region is not solely the accounts array — it also contains RISK_BUF (160 bytes) and a per-account generation table (N × 8 bytes). The function therefore over-reports by exactly 6 slots on every tier.

parseAccount() uses this over-reported value as its only range guard, rather than the layout.maxAccounts available on the object it obtained one line earlier. Indices in the surplus range are accepted and return a fully-populated Accountowner, capital, positionSize, pnl — decoded from generation-table and risk-buffer bytes. Nothing throws, and every field parses cleanly, so there is no signal that the data is meaningless.


2. Affected locations

2.1 The defective computation — src/solana/slab.ts:3481–3487

/**
 * Calculate the maximum valid account index for a given slab size.
 */
export function maxAccountIndex(dataLen: number): number {
  const layout = detectSlabLayout(dataLen);
  if (!layout) return 0;
  const accountsEnd = dataLen - layout.accountsOff;
  if (accountsEnd <= 0) return 0;
  return Math.floor(accountsEnd / layout.accountSize);   // <-- line 3486
}

2.2 The consumer that trusts it as a bound — src/solana/slab.ts:3492–3499

export function parseAccount(data: Uint8Array, idx: number): Account {
  const layout = detectSlabLayout(data.length, data);     // <-- layout.maxAccounts available here
  if (!layout) throw new Error(`Unrecognized slab data length: ${data.length}`);

  const maxIdx = maxAccountIndex(data.length);            // <-- but this is used instead
  if (!Number.isInteger(idx) || idx < 0 || idx >= maxIdx) {
    throw new Error(`Account index out of range: ${idx} (max: ${maxIdx - 1})`);
  }

2.3 The bulk parser, filtered against the same wrong value — src/solana/slab.ts:4934–4948

export function parseAllAccounts(data: Uint8Array): { idx: number; account: Account }[] {
  const indices = parseUsedIndices(data);
  const maxIdx = maxAccountIndex(data.length);
  const validIndices = indices.filter(idx => idx < maxIdx);
  const droppedCount = indices.length - validIndices.length;
  if (droppedCount > 0) {
    console.warn(
      `[parseAllAccounts] bitmap claims ${indices.length} used accounts but only ${maxIdx} fit ` +
      `in the slab — ${droppedCount} out-of-bounds indices dropped (possible bitmap corruption)`,
    );
  }

Note the intent visible here: this warning exists specifically to catch out-of-range bitmap entries. Because it compares against the inflated ceiling, it does not fire for the six surplus slots — the guard is defeated by the same defect it was written to catch.


3. Root cause

3.1 The divisor covers more than the accounts array

A v12.19 slab does not end at the accounts array. Per the SDK's own layout builder (buildLayoutV12_19, slab.ts:988–1027) and size table (V12_19_SIZES, slab.ts:968–973), total length is:

SLAB_LEN = ENGINE_OFF
         + accountsOffRel        (bitmap + num_used + free_head + next_free + prev_free + padding)
         + N × ACCOUNT_SIZE      (the accounts array)
         + RISK_BUF_LEN          (160 bytes)
         + N × GEN_TABLE_ENTRY   (8 bytes per account)

maxAccountIndex() divides RISK_BUF_LEN + N × 8 — bytes belonging to neither the accounts array nor any account — by accountSize and reports the quotient as additional slots.

For the small tier: 160 + 256 × 8 = 2 208 surplus bytes; 2 208 / 360 = 6.13, floored to 6 phantom slots.

3.2 The over-count is uniform across every tier

Tier dataLen layout.maxAccounts accountsOff maxAccountIndex() Over-count
micro 26 872 64 1 624 70 +6
small 96 784 256 2 416 262 +6
medium 376 432 1 024 5 584 1 030 +6
large 1 495 024 4 096 18 256 4 102 +6

The small tier is the layout slab.ts:970 records as "probe-confirmed; deployed mainnet ESa89R5…".

3.3 The correct bound was already computed and available

parseAccount() obtains layout on its first line. layout.maxAccounts is the authoritative slot count, derived from the same constants that produced accountsOff and accountSize. The function reaches past it for a helper that re-derives a weaker answer. This is the whole defect: not a wrong constant, but the wrong variable.

3.4 A contributing naming hazard

maxAccountIndex returns a count, not a maximum index. parseAccount treats it as an exclusive upper bound (idx >= maxIdx) while its own error message prints maxIdx - 1. The name invites exactly the confusion that would make a reviewer accept it as a bound.


4. Scope note on cross-validation

percolator-prog (the v17 wrapper) contains no slab concept at all — zero occurrences of SLAB_LEN, RISK_BUF, GEN_TABLE, or MAX_ACCOUNTS — and its Cargo.toml explicitly declares the tier features to be no-ops:

# v16 is account-local and has no slab-wide account-count tier. These feature names are no-ops…

The v12.19 layout constants therefore originate from the separate percolator engine crate (path = "../percolator"), which was not available for this audit.

This does not weaken the finding. The defect is an internal inconsistency: whatever the correct layout constants are, maxAccountIndex() must never exceed the layout.maxAccounts the SDK itself computed from those same constants. The proof of concept below demonstrates the inconsistency using only the SDK's own values, and the fix is expressed in terms of them.


5. Proof of concept

Self-contained. Save as poc-sdk-03.mjs in the repository root after npm install.

// poc-sdk-03.mjs — run with: node poc-sdk-03.mjs
import { detectSlabLayout, maxAccountIndex, parseAccount } from './dist/index.js';

const DATA_LEN = 96784;                       // v12.19 --features small (probe-confirmed)
const layout = detectSlabLayout(DATA_LEN);

console.log('layout.maxAccounts   =', layout.maxAccounts);
console.log('layout.accountSize   =', layout.accountSize);
console.log('layout.accountsOff   =', layout.accountsOff);
console.log('maxAccountIndex(len) =', maxAccountIndex(DATA_LEN));
console.log('over-count           =', maxAccountIndex(DATA_LEN) - layout.maxAccounts, 'phantom slots');

// Build a synthetic slab: valid magic, everything zeroed, and 0xAB poison written
// ONLY into the region beyond the real accounts array (RISK_BUF + generation table).
// Anything that decodes as 0xABAB... therefore came from outside the accounts array.
const buf = new Uint8Array(DATA_LEN);
new DataView(buf.buffer).setBigUint64(0, 0x504552434f4c4154n, true);   // "PERCOLAT"
const realEnd = layout.accountsOff + layout.maxAccounts * layout.accountSize;
buf.fill(0xAB, realEnd, DATA_LEN);

console.log('\nreal accounts array ends at byte', realEnd, 'of', DATA_LEN);
console.log('trailing bytes (RISK_BUF + gen table):', DATA_LEN - realEnd, '\n');

for (const idx of [layout.maxAccounts - 1, layout.maxAccounts, maxAccountIndex(DATA_LEN) - 1]) {
  const beyond = idx >= layout.maxAccounts;
  try {
    const a = parseAccount(buf, idx);
    console.log(`parseAccount(idx=${idx})${beyond ? '   <-- BEYOND layout.maxAccounts' : ''}`);
    console.log(`   -> OK  owner=${a.owner.toBase58().slice(0, 20)}...  capital=${a.capital}`);
  } catch (e) {
    console.log(`parseAccount(idx=${idx}) -> threw: ${e.message}`);
  }
}

Actual output (Node 26, SDK c976188):

layout.maxAccounts   = 256
layout.accountSize   = 360
layout.accountsOff   = 2416
maxAccountIndex(len) = 262
over-count           = 6 phantom slots

real accounts array ends at byte 94576 of 96784
trailing bytes (RISK_BUF + gen table): 2208

parseAccount(idx=255)
   -> OK  owner=11111111111111111111...  capital=0
parseAccount(idx=256)   <-- BEYOND layout.maxAccounts
   -> OK  owner=CZ8YUVdk7znjrUmnb5n7...  capital=228189351935217557851910030866009271211
parseAccount(idx=261)   <-- BEYOND layout.maxAccounts
   -> OK  owner=CZ8YUVdk7znjrUmnb5n7...  capital=228189351935217557851910030866009271211

Reading the result:

  • Index 255 is the last real slot and correctly returns zeros — the control case.
  • Indices 256 and 261 lie beyond layout.maxAccounts. Both return an Account whose owner is a well-formed base58 public key and whose capital is 0xABAB… reinterpreted as a u128 — the poison bytes, proving the read came from outside the accounts array.
  • No exception is raised at any point. The caller has no way to distinguish these from real accounts.

5.1 Verifying the over-count across all four tiers

// poc-sdk-03-tiers.mjs
import { detectSlabLayout, maxAccountIndex } from './dist/index.js';
for (const len of [26872, 96784, 376432, 1495024]) {
  const l = detectSlabLayout(len);
  const mi = maxAccountIndex(len);
  console.log(
    `dataLen=${String(len).padStart(7)}  maxAccounts=${String(l.maxAccounts).padStart(4)}` +
    `  maxAccountIndex=${String(mi).padStart(4)}  over-count=+${mi - l.maxAccounts}`,
  );
}

Actual output:

dataLen=  26872  maxAccounts=  64  maxAccountIndex=  70  over-count=+6
dataLen=  96784  maxAccounts= 256  maxAccountIndex= 262  over-count=+6
dataLen= 376432  maxAccounts=1024  maxAccountIndex=1030  over-count=+6
dataLen=1495024  maxAccounts=4096  maxAccountIndex=4102  over-count=+6

6. Impact

Callers receive syntactically valid Account objects containing arbitrary bytes. Because every field decodes without error, nothing signals that the data is meaningless:

  • owner is a PublicKey built from 32 bytes of generation-table data — a well-formed but meaningless address that a UI or indexer could attribute to a real user.
  • capital, positionSize, pnl, and reservedPnl are large arbitrary integers.
  • Any aggregate computed over parsed accounts — total open interest, insurance coverage, ADL ranking, risk dashboards — is silently corrupted by the phantom rows.

Reachability.

  • Direct parseAccount(data, idx) callers have no protection at all. Any index in [maxAccounts, maxAccountIndex) succeeds.
  • parseAllAccounts() is partially protected in practice, because it iterates only indices set in the used-bitmap, which is layout.bitmapWords × 64 bits wide. However its own filter uses the inflated ceiling, so a corrupt or adversarially-crafted bitmap with bits set in the surplus range passes straight through — and the console.warn written to catch exactly that never fires.

This is not remotely exploitable on its own: it requires a caller to request an out-of-range index, or a slab whose bitmap is already corrupt. It is a correctness and data-integrity defect in a parsing path that downstream risk tooling depends on.


7. Remediation

7.1 Primary fix — clamp to the layout

--- a/src/solana/slab.ts
+++ b/src/solana/slab.ts
@@ -3478,10 +3478,14 @@
 /**
- * Calculate the maximum valid account index for a given slab size.
+ * Number of parseable account slots for a given slab size.
+ *
+ * Bounded by `layout.maxAccounts`: on v12.17+ layouts the region after
+ * `accountsOff` also holds RISK_BUF (160 B) and the per-account generation
+ * table (N x 8 B), so the raw byte division over-reports by several slots.
  */
 export function maxAccountIndex(dataLen: number): number {
   const layout = detectSlabLayout(dataLen);
   if (!layout) return 0;
   const accountsEnd = dataLen - layout.accountsOff;
   if (accountsEnd <= 0) return 0;
-  return Math.floor(accountsEnd / layout.accountSize);
+  return Math.min(layout.maxAccounts, Math.floor(accountsEnd / layout.accountSize));
 }

7.2 Defence in depth — bound against the layout directly

parseAccount() should assert against the object it already holds rather than delegating to a helper. This is strictly more robust and removes a redundant detectSlabLayout() call (see §7.3):

--- a/src/solana/slab.ts
+++ b/src/solana/slab.ts
@@ -3492,9 +3492,9 @@
 export function parseAccount(data: Uint8Array, idx: number): Account {
   const layout = detectSlabLayout(data.length, data);
   if (!layout) throw new Error(`Unrecognized slab data length: ${data.length}`);
 
-  const maxIdx = maxAccountIndex(data.length);
-  if (!Number.isInteger(idx) || idx < 0 || idx >= maxIdx) {
-    throw new Error(`Account index out of range: ${idx} (max: ${maxIdx - 1})`);
+  if (!Number.isInteger(idx) || idx < 0 || idx >= layout.maxAccounts) {
+    throw new Error(`Account index out of range: ${idx} (max: ${layout.maxAccounts - 1})`);
   }

7.3 Optional — resolve the naming hazard, and a related performance issue

maxAccountIndex returning a count invites the confusion that produced this defect. Renaming it, with a deprecated alias, removes the hazard for future callers:

/** Number of parseable account slots. (Renamed from `maxAccountIndex`, which returned a count, not an index.) */
export function accountSlotCount(dataLen: number): number { /* ... */ }

/** @deprecated Misleading name — returns a COUNT, not a maximum index. Use {@link accountSlotCount}. */
export const maxAccountIndex = accountSlotCount;

Separately, parseAllAccounts() currently triggers two detectSlabLayout() calls per account (one direct, one via maxAccountIndex) — 8 194 detections on a full 4 096-slot slab, about 5 ms of 19 ms. Threading the layout through resolves both issues at once:

-export function parseAccount(data: Uint8Array, idx: number): Account {
-  const layout = detectSlabLayout(data.length, data);
+export function parseAccount(data: Uint8Array, idx: number, layoutHint?: SlabLayout | null): Account {
+  const layout = layoutHint ?? detectSlabLayout(data.length, data);
 export function parseAllAccounts(data: Uint8Array): { idx: number; account: Account }[] {
+  const layout = detectSlabLayout(data.length, data);
+  if (!layout) throw new Error(`Unrecognized slab data length: ${data.length}`);
   const indices = parseUsedIndices(data);
-  const maxIdx = maxAccountIndex(data.length);
+  const maxIdx = layout.maxAccounts;
   const validIndices = indices.filter(idx => idx < maxIdx);
   ...
   return validIndices.map(idx => ({
     idx,
-    account: parseAccount(data, idx),
+    account: parseAccount(data, idx, layout),
   }));
 }

This is the layoutHint pattern the module already uses for parseConfig() (slab.ts:2819) and parseParams() (slab.ts:3060).

7.4 Regression tests

it.each([26872, 96784, 376432, 1495024])(
  'maxAccountIndex never exceeds layout.maxAccounts (dataLen=%i)',
  (len) => {
    const layout = detectSlabLayout(len)!;
    expect(maxAccountIndex(len)).toBeLessThanOrEqual(layout.maxAccounts);
  },
);

it('parseAccount rejects indices at or beyond maxAccounts', () => {
  const DATA_LEN = 96784;
  const layout = detectSlabLayout(DATA_LEN)!;
  const buf = new Uint8Array(DATA_LEN);
  new DataView(buf.buffer).setBigUint64(0, 0x504552434f4c4154n, true);
  expect(() => parseAccount(buf, layout.maxAccounts)).toThrow(/out of range/);
  expect(() => parseAccount(buf, layout.maxAccounts + 5)).toThrow(/out of range/);
});

it('does not read past the accounts array', () => {
  const DATA_LEN = 96784;
  const layout = detectSlabLayout(DATA_LEN)!;
  const buf = new Uint8Array(DATA_LEN);
  new DataView(buf.buffer).setBigUint64(0, 0x504552434f4c4154n, true);
  const realEnd = layout.accountsOff + layout.maxAccounts * layout.accountSize;
  buf.fill(0xAB, realEnd, DATA_LEN);
  // The last real slot must be untouched by the poison.
  expect(parseAccount(buf, layout.maxAccounts - 1).capital).toBe(0n);
});

8. Fix verification

The §7.1 patch was applied to a clean clone of c976188, compiled, and tested.

Check Result
npm run build success
PoC before fix parseAccount(idx=256) -> OK owner=CZ8YUVdk7znjrUmnb5n7... capital=228189351935217557851910030866009271211
PoC after fix parseAccount(idx=256) -> threw: Account index out of range: 256 (max: 255)
npm test baseline 29 files, 1 008 passed, 31 skipped, exit 0
npm test after fix 29 files, 1 008 passed, 31 skipped, exit 0

Reproduce end to end:

git clone https://github.com/dcccrypto/percolator-sdk && cd percolator-sdk
git checkout c976188 && npm install
npm test                          # baseline: 1008 passed

node poc-sdk-03.mjs               # idx=256 returns a fabricated account

# apply §7.1, then:
npm run build
node poc-sdk-03.mjs               # idx=256 now throws "Account index out of range"
npm test                          # 1008 passed — no regression

9. References

  • src/solana/slab.ts:3481–3487maxAccountIndex
  • src/solana/slab.ts:3492–3499parseAccount bounds check
  • src/solana/slab.ts:4934–4948parseAllAccounts filter and its defeated warning
  • src/solana/slab.ts:988–1027buildLayoutV12_19
  • src/solana/slab.ts:968–973V12_19_SIZES, incl. the mainnet-deployed small tier
  • src/solana/slab.ts:2819, 3060 — the existing layoutHint pattern
  • percolator-prog/Cargo.toml — confirms the v17 wrapper has no slab tiering
  • CWE-125: Out-of-bounds Read
  • CWE-1284: Improper Validation of Specified Quantity in Input

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions