Skip to content

fix(yield_vault): bind initialize to the contract's deployer - #26

Open
hugrug wants to merge 1 commit into
YieldAnchor:mainfrom
hugrug:fix/issue-8
Open

hugrug wants to merge 1 commit into
YieldAnchor:mainfrom
hugrug:fix/issue-8

Conversation

@hugrug

@hugrug hugrug commented Sep 29, 2026

Copy link
Copy Markdown

Fixes #8

Problem

initialize only required the proposed admin to sign. On a freshly created vault, a third party reaching initialize before the deployer could name itself as admin and sign its own call. It would then control pause/unpause permanently, since there is no way to replace an admin. The deploy scripts hid the window by initializing right after creation, but the contract itself did nothing to close it.

Fix

  • initialize(deployer, salt, admin, asset, name, symbol, decimals) re-derives the contract's own address from deployer and the 32-byte creation salt. If the result is not this contract, it returns the new VaultError::BadDeployer (17). deployer must authorize the call, and so must admin when it is a different address. These checks run before metadata validation, and AlreadyInit still prevents re-initialization.
  • createVaultContract now always passes an explicit salt (random unless one is given) and returns it. YieldVaultClient.initialize and the validation schema take deployer and a hex salt. BadDeployer is added to the error constants and messages.
  • Both deploy scripts pass the deployer and salt to initialize. They also write the salt to scripts/.deploy_salt right after creation, so the vault can still be initialized if the run fails later. deploy-yield-vault.sh accepts DEPLOY_SALT from the environment to redeploy at a known address. It checks that the value is exactly 64 hex characters and stops with an error before building anything if it isn't; otherwise it generates a random salt as before.
  • contracts/yield_vault/README.md and docs/deployment/testnet.md describe the new guarantee. The testnet doc replaces the old caveat, explains how to initialize later from the saved salt, and notes that a lost salt can be recovered from the vault's creation transaction.

How it was verified

  • cargo test -p yield_vault: 17 tests pass. They include:
    • front_runner_cannot_capture_admin_of_a_fresh_vault: an attacker that signs its own call is rejected with BadDeployer, then the deployer initializes, then the attacker gets AlreadyInit.
    • A wrong salt is rejected, even when the real deployer signs.
    • A separate admin needs both signatures.
    • A G... account deployer can initialize.
    • The existing initialization tests.
  • Where initialize fails for a missing signature (no signature, deployer's missing, or separate admin's missing), the tests assert that the host rejected it with Error(Auth, InvalidAction), not just that it errored. A try_ call can't show this, because the host narrows non-contract failures to Error(Context, InvalidAction). So these cases invoke without try_ and check the host error they fail with.
  • cargo clippy -p yield_vault --all-targets -- -D warnings and cargo fmt --all -- --check are clean.
  • The @yieldanchor/contract-clients and @yieldanchor/validation tests pass. eslint reports no errors and prettier --check passes on the changed docs.
  • The DEPLOY_SALT check in deploy-yield-vault.sh was exercised locally with empty, short, 65-character, non-hex and upper-case hex values.
  • The deploy scripts were not run against Testnet.
  • The repository's CI workflow passed on this branch: https://github.com/hugrug/yield-general/actions/runs/36515866656

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.

fix: initialize is unauthenticated, so a front-runner can capture admin on a fresh vault

1 participant