Skip to content

fix(token): bypass cache for payment-critical account reads#418

Merged
dev-jodee merged 11 commits intomainfrom
fix/audit-findings
Apr 7, 2026
Merged

fix(token): bypass cache for payment-critical account reads#418
dev-jodee merged 11 commits intomainfrom
fix/audit-findings

Conversation

@dev-jodee
Copy link
Copy Markdown
Contributor

@dev-jodee dev-jodee commented Apr 7, 2026

Summary

  • force-refresh destination token account data in TokenUtil::find_payment_in_transaction
  • force-refresh Token-2022 mint account data used for transfer-fee calculation in the same path
  • remove stale Redis account-cache reads from payment authorization and pricing decisions

Test Plan (if applicable)

  • cargo test -p kora-lib tests_token::
  • cargo test -p kora-lib validate_token2022_extensions_for_payment -- --nocapture

Breaking Changes (if applicable)

  • none

Open with Devin

📊 Unit Test Coverage

Coverage

Unit Test Coverage: 84.5%

View Detailed Coverage Report

Bypass Redis account cache for destination and mint lookups in
TokenUtil::find_payment_in_transaction to avoid stale account
state during payment authorization and pricing.
@dev-jodee dev-jodee requested a review from amilz as a code owner April 7, 2026 15:39
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

📊 TypeScript Coverage Report

Coverage: 33.9%

View detailed report

Coverage artifacts have been uploaded to this workflow run.
View Artifacts

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 7, 2026

Greptile Summary

This fix ensures the payment-authorization path in TokenUtil::find_payment_in_transaction always reads live on-chain state instead of potentially stale Redis cache entries. The two changes — force-refreshing the destination token account and the Token-2022 mint account — prevent scenarios where stale cache data could cause incorrect destination-owner validation or wrong transfer-fee calculations, which could lead to under-collected fees or bypassed payment checks.

Confidence Score: 5/5

Safe to merge — two targeted boolean flips on the payment-critical path, no regressions expected

No P0 or P1 issues found. The fix is minimal and correct: force_refresh=true fetches from RPC and repopulates the cache, so there is no data loss. All other false usages in the codebase remain appropriate for their non-authoritative (fee-estimation/read-only) contexts.

No files require special attention

Important Files Changed

Filename Overview
crates/lib/src/token/token.rs Two force_refresh flags changed from false→true in find_payment_in_transaction; destination token account and Token-2022 mint are now always fetched live from RPC on the payment-authorization path

Sequence Diagram

sequenceDiagram
    participant C as Client
    participant V as TransactionValidator
    participant T as TokenUtil
    participant Cache as CacheUtil
    participant RPC as Solana RPC
    participant Redis as Redis Cache

    C->>V: signTransaction / signAndSendTransaction
    V->>T: verify_token_payment(required_lamports, expected_dest)
    T->>T: find_payment_in_transaction()
    loop For each SplTokenTransfer
        T->>Cache: get_account(destination_address, force_refresh=true)
        Cache->>RPC: get_account (live read, bypasses cache)
        RPC-->>Cache: Account data
        Cache->>Redis: repopulate cache
        Cache-->>T: destination Account
        T->>T: unpack → (owner, mint)
        alt Token-2022
            T->>Cache: get_account(token_mint, force_refresh=true)
            Cache->>RPC: get_account (live read, bypasses cache)
            RPC-->>Cache: Mint account data
            Cache->>Redis: repopulate cache
            Cache-->>T: Mint Account
            T->>T: calculate_transfer_fee → effective_amount
        end
        T->>T: convert to lamports via price oracle
    end
    T-->>V: total_lamport_value
    V-->>C: Ok(authorized) or Err(insufficient payment)
Loading

Reviews (1): Last reviewed commit: "fix(token): force fresh account reads in..." | Re-trigger Greptile

Reject Token-2022 payment mints that carry a mutable TransferHook authority so post-sign hook updates cannot alter the execution surface Kora approved.

Apply the same guard for destination ATA-creation flows and extend mint test builders to configure TransferHook authority/program fields for coverage.
@dev-jodee
Copy link
Copy Markdown
Contributor Author

Audit finding fix: Mutable Token-2022 TransferHook state post-sign update\n\nImplemented in commit dc3476d.\n\nWhat changed:\n- Added a payment-time guard that rejects Token-2022 mints when the TransferHook extension is present with a non-null authority (i.e. hook program is still mutable).\n- Applied the same guard in both validation paths:\n - validate_token2022_extensions_for_payment\n - validate_token2022_partial_for_ata_creation (ATA created in-flight)\n- Added targeted unit tests for reject/allow behavior and ATA-creation path coverage.\n\nSecurity impact:\n- Prevents post-sign TransferHook program swaps from changing the effective inner CPI surface after Kora has already approved/signatured the transaction.

Run Token-2022 extension and transfer-hook authority validation only after destination-owner matching in find_payment_in_transaction.

This avoids rejecting unrelated Token-2022 transfers that are not payment instructions while preserving the mutable transfer-hook guard for actual payment paths.
Restrict signer position lookup to the first num_required_signatures account keys so unsigned pubkey occurrences cannot be treated as signer slots.

Also switch both normal and bundle signing flows to checked signature-slot writes and return InvalidTransaction on slot mismatch instead of panicking.

Add coverage for rejecting unsigned fee payer occurrences in bundle signing tests.
Extend parsed system and SPL/Token-2022 instruction data to retain destination authority/owner pubkeys carried in instruction bytes (e.g. AuthorizeNonceAccount new_authority, SetAuthority new_authority, InitializeMint freeze_authority).

Add a dedicated disallowed-instruction-data validation pass and run it during transaction validation so blacklisted pubkeys in instruction data are rejected the same way as account metas/program IDs.

Includes new validator tests for nonce authorize, SPL/Token-2022 set_authority, initialize_account2 owner, and initialize_mint2 freeze_authority bypass paths.
Adjust SPL transfer value accounting so Token-2022 inflows to fee-payer-owned
accounts are credited at post-fee amounts, while outflows remain gross.

This closes the transfer-fee over-credit path where fee payer outflow was
underestimated against max_allowed_lamports.
Reject confidential Token-2022 extension instruction families by default and
add explicit Token-2022 Reallocate parsing.

Also reject fee-payer-involved Token-2022 Reallocate usage in transaction
validation to close fee-payer policy bypass paths.
devin-ai-integration[bot]

This comment was marked as resolved.

Fold instruction-data disallowed checks into validate_disallowed_accounts so
account-meta and argument-level checks run in a single validation path.

Also include SystemInitializeNonceAccount.nonce_authority in the disallowed
instruction-data coverage.
Replace function-local TransferValue struct with tuple storage in
calculate_spl_transfers_value_in_lamports to match existing local style
patterns in this codebase without changing behavior.
@dev-jodee dev-jodee merged commit 6151e75 into main Apr 7, 2026
12 checks passed
@dev-jodee dev-jodee deleted the fix/audit-findings branch April 7, 2026 20:12
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.

1 participant