Skip to content

fix(contracts): close front-runnable adapter initialize() with __constructor - #550

Merged
collinsezedike merged 3 commits into
drydocs:mainfrom
blockchain-maxis:fix/adapter-constructor-front-run
Aug 23, 2026
Merged

fix(contracts): close front-runnable adapter initialize() with __constructor#550
collinsezedike merged 3 commits into
drydocs:mainfrom
blockchain-maxis:fix/adapter-constructor-front-run

Conversation

@blockchain-maxis

Copy link
Copy Markdown
Contributor

closes #505

Summary

Both adapters set their vault/protocol/USDC addresses in an unauthenticated initialize(), guarded only against re-initialization. Since deploy and initialize were two separate transactions, anyone watching the ledger could land initialize() first on a freshly deployed adapter with their own address as vault, becoming the only party able to move funds through it, and the legitimate call would then fail with AlreadyInitialized.

Both adapters now set that state in __constructor, which the host runs inside the CreateContract operation that deploys the contract. There is no intervening ledger left to race.

As agreed in the issue thread, require_auth() on a caller-supplied admin parameter is not the fix here. It would only prove the racer controls the address they chose to pass in, which is trivially true, and says nothing about transaction ordering. Nothing in this PR adds one.

SDK support confirmed before starting, as asked: soroban-sdk is pinned at 22.0.0, which resolves to 22.0.11 in packages/contracts/Cargo.lock. That version supports __constructor (soroban-sdk-22.0.11/src/env.rs, and ContractArgs::__constructor for env.register in tests). Both WASMs export __constructor after this change, verified from stellar contract build's own export listing.

What changed

4a83ca5 — contracts. MeridianBlendAdapter and MeridianDefindexAdapter each get a __constructor(vault, pool|defindex_vault, usdc).

initialize() is deliberately kept. It leaves the ABI of adapters already deployed from older WASM unchanged, and they can still be initialized by hand. On anything deployed from this WASM it always returns AlreadyInitialized, because __constructor has already written VAULT_KEY. Both entry points delegate to a private, unexported init_state() so the two can never set up different state.

faf6c3d — deploy scripts. deploy-testnet.sh and redeploy-blend-adapter.sh pass the wiring to stellar contract deploy after a -- separator instead of invoking initialize() afterwards. deploy-testnet.sh's deploy() helper now forwards trailing arguments, so the vault (no constructor) and the adapter (three constructor args) share one helper.

redeploy-blend-adapter.sh now requires VAULT_ID rather than defaulting to a hardcoded address. That default was already stale (CBK5RI4B..., not the live vault), and the vault address is now baked into the adapter by the deploying transaction and cannot be changed afterwards, so a stale default would permanently bind a fresh adapter to the wrong vault with a redeploy as the only way out. Flagging this as a small interface change, since it is technically beyond the letter of the issue but is a direct consequence of making the binding permanent.

fdd80d3 — docs. New "Adapter deployment and initialization" section in apps/docs/architecture/vault-contract.md covering the constructor, the CLI syntax, why there is no separate initialize step, and why initialize() is still present but unreachable on new deploys. apps/docs/operations/testnet-deployment.md updated to match.

Tests

Both test suites now register their adapter through the constructor, so the entire existing suite runs against the deployment path real contracts use rather than a path that no longer exists. Added per adapter:

  • constructor_sets_vault_*_and_usdc — asserts the constructor wrote vault, protocol contract, and USDC, with no initialize() call anywhere in setup.
  • initialize_cannot_hijack_a_constructor_deployed_adapter — replays the [Bug] Adapter initialize() is unauthenticated, front-runnable #505 front-run against the fixed contract: an attacker calls initialize() with their own address as vault, gets AlreadyInitialized, and the adapter is asserted to still be bound to the real vault.

The existing reinitializing_fails tests are kept and now cover the same guard from the constructor-deployed side.

Verification performed

  • cargo test --release in packages/contracts: 69 passed, 0 failed (blend-adapter 14, defindex-adapter 12, vault 43). Adapter counts are up from 12 and 10.
  • cargo fmt --all -- --check: clean.
  • cargo clippy --all-targets -- -D warnings: clean.
  • stellar contract build: both adapter WASMs build and list __constructor among their exported functions.
  • bash -n on both modified scripts: clean.
  • pnpm format:check: clean.
  • The live testnet BlendAdapter (CDFIDKNA2ZTB37I7RN32WH7VU5AP2PAOXLGFWMTW6T2RSUM23AJIV2YM) was checked directly: simulating initialize() against it returns Error(Contract, #1) (AlreadyInitialized), so it is already initialized and not claimable. No redeploy is forced by this PR; the fix applies to every future adapter deployment.

Scope notes

The vault has the same bug, and is not fixed here. MeridianVault::initialize() is equally callable by anyone (admin.require_auth() only checks the admin the caller passed in), and set_adapter is gated only on total_shares > 0, so a freshly deployed, uninitialized vault can be claimed and repointed before the first deposit. I raised this on #544 and asked whether to fold the vault into this PR; that thread did not get a reply before #544 merged, so I have kept this PR to the two adapters the issue actually names.

It is not a mechanical addition either: the vault has eight tests (get_admin_fails_before_initialize, deposit_fails_before_initialize, and so on) that register an uninitialized vault on purpose, and a constructor would make that state unreachable and NotInitialized dead. That is worth its own issue and its own review rather than being appended here. Happy to open one, or to add it to this PR if you would rather have it in one go.

scripts/deploy-testnet.sh's ADMIN_KEY path from #544 stays as it is: it is the interim mitigation for that vault-side window and remains correct until the vault gets the same constructor treatment.

Not run / pre-existing: pnpm install --frozen-lockfile still fails with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH because the locally available pnpm (10.22.0) does not match the repo's pinned packageManager (pnpm@9.0.0). Unrelated to this change and not bundled here; pnpm-lock.yaml is untouched.

BlendAdapter::initialize() and DefindexAdapter::initialize() set the
adapter's vault, protocol, and USDC addresses with no authorization
check, guarded only against re-initialization. Because deploy and
initialize were two separate transactions, anyone watching the ledger
could land initialize() first on a freshly deployed adapter with their
own address as vault, becoming the only party able to move funds
through it. The legitimate call then failed with AlreadyInitialized.

Adding require_auth() to initialize() does not fix this. It would only
prove the racer controls the address they chose to pass in, which is
trivially true, and says nothing about transaction ordering. The window
itself has to go.

Both adapters now set that state in __constructor, which the host runs
inside the CreateContract operation that deploys the contract, in the
same transaction. There is no intervening ledger left to race.
soroban-sdk is pinned at 22.0.0 (resolves to 22.0.11), which supports
__constructor; both WASMs export it after this change.

initialize() is kept so the ABI of adapters already deployed from older
WASM is unchanged and they can still be initialized by hand. On
anything deployed from this WASM it always returns AlreadyInitialized,
since __constructor has already written VAULT_KEY. Both entry points
share a private init_state() so they cannot set up different state.

Tests register both adapters through the constructor now, so the whole
existing suite runs against the deployment path real contracts use.
Added, per adapter: a test that the constructor wrote vault, protocol
and USDC, and a test that replays the front-run against the fixed
contract and asserts the adapter stays bound to the real vault.
Both deploy scripts deployed the adapter and then initialized it in a
separate transaction, which is the window drydocs#505 is about. They now pass
vault/pool/USDC to stellar contract deploy after a -- separator, so the
adapter is wired by the transaction that creates it, and the separate
initialize() invoke is gone.

deploy-testnet.sh's deploy() helper forwards any trailing arguments to
stellar contract deploy, so the vault (no constructor) and the adapter
(three constructor args) share one helper.

redeploy-blend-adapter.sh now requires VAULT_ID instead of defaulting to
a hardcoded address. That default was already stale, and the vault
address is now written into the adapter by the deploying transaction
and cannot be changed afterwards, so a stale default would permanently
bind a fresh adapter to the wrong vault with a redeploy as the only way
out.
Adds an "Adapter deployment and initialization" section to the vault
contract architecture doc covering the constructor, the CLI syntax for
passing its arguments, why there is no separate initialize step, and
why initialize() is still present but unreachable on new deploys.

Updates the testnet deployment walkthrough to match, and notes that
redeploy-blend-adapter.sh now requires VAULT_ID.
@vercel

vercel Bot commented Aug 23, 2026

Copy link
Copy Markdown

@blockchain-maxis is attempting to deploy a commit to the Collins' projects Team on Vercel.

A member of the Team first needs to authorize it.

@collinsezedike

Copy link
Copy Markdown
Collaborator

@blockchain-maxis thank you for the __constructor fix, and for flagging that the vault has the same bug rather than leaving it implicit. Agreed on keeping this PR scoped to the two adapters, the vault fix needs its own test-suite rework, not something to fold in here.

Filed the vault-side gap as #551, tracking it separately.

Merging now.

@collinsezedike

Copy link
Copy Markdown
Collaborator

@blockchain-maxis if you'd like to take on #551 as well, it's the same fix pattern you just proved out here, just applied to the vault with the added work of reworking the tests that assume an uninitialized vault state.

@collinsezedike
collinsezedike merged commit d73cffc into drydocs:main Aug 23, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Adapter initialize() is unauthenticated, front-runnable

2 participants