feat(sdk-coin-sol): add verifyTransaction validation for staking authorize intent

[bitgo-ai-agent-dev]

What

• Added isStakingAuthorizeTx branch in sol.ts:verifyTransaction that validates the decoded SOL staking authorize instruction against the caller's intent fields:
  • oldWithdrawAddress matches the wallet root address (Withdrawer authority only; skipped for staker-only txs where the field is '')
  • newWithdrawAddress matches txParams.newWithdrawPublicKey from the intent (when set)
  • stakingAddress matches txParams.stakeAccount from the intent (when set)
• Extended resolveEffectiveTxParams in sdk-core to propagate newWithdrawPublicKey and stakeAccount from txRequest.intent into txParams so the fields reach verifyTransaction through the TSS signing flow
• Added newWithdrawPublicKey and stakeAccount to PopulatedIntent (baseTypes.ts) and TransactionParams (iBaseCoin.ts)
• Fixed explainTransaction for both the WASM (tsol) and non-WASM (sol) paths to populate stakingAuthorize with the Withdrawer instruction's fields (the security-critical one), not the Staker instruction's, when a standard two-instruction authorize tx is present
• Added unit tests covering: happy path with all intent fields, happy path without optional fields, malicious newWithdrawPublicKey substitution, wrong stakeAccount, wrong oldWithdrawAddress, non-WASM explainTransaction path, and recipientUtils propagation

Why

• 'authorize' was added to NO_RECIPIENT_TX_TYPES (WCI-1111 / PR fix(sdk-core): wire resolveEffectiveTxParams into EddsaMPCv2Utils (WCI-1111) #9394) because SolAuthorizeIntent has empty recipients by design. Without coin-layer validation, a compromised BitGo server could present a StakingAuthorize txHex that rotates the stake account's withdraw authority to an attacker-controlled key, and the client would sign it with no checks.
• This closes the security gap identified in TOB-BITGOEDMPC-1 (Trail of Bits security audit) and explicitly flagged in WCI-1110 scoping notes as requiring coin-layer validation before the signing flow could be considered safe for authorize intents.

Test plan

• yarn unit-test --scope @bitgo/sdk-coin-sol — 646 tests pass
• yarn unit-test --scope @bitgo/sdk-core (recipientUtils tests) — 32 tests pass
• Type checks pass for both sdk-coin-sol and sdk-core
• Happy path: valid authorize tx with matching newWithdrawPublicKey and stakeAccount passes
• Malicious case: swapped newWithdrawPublicKey correctly throws
• Malicious case: wrong stakeAccount correctly throws
• Wrong oldWithdrawAddress (wallet root mismatch) correctly throws
• Staker-only authorize tx (empty oldWithdrawAddress) does not block legitimate staker-authority changes

Ticket: CHALO-1294

[linear-code]

CHALO-1294

VL-7606

[maheshbitgo]

@claude Review the changes for security, correctness and adversarial effects/regression

[github-actions]

Claude finished @maheshbitgo's task —— View job

---

Security and Correctness Review

I've completed a comprehensive security review of the Solana staking authorize transaction validation changes. Here's my detailed analysis:

Todo List:

• Read and analyze all modified files
• Review security validation logic in sol.ts:verifyTransaction
• Examine parameter propagation in sdk-core modules
• Analyze test coverage for malicious scenarios
• Check for potential bypass methods or regressions
• Provide comprehensive security assessment

---

Security Assessment: ✅ APPROVED

This PR effectively addresses the Trail of Bits audit finding (TOB-BITGOEDMPC-1) by implementing comprehensive validation for Solana staking authorize transactions.

Key Security Strengths

1. Critical Vulnerability Mitigation ✅

• Problem: Previously, authorize transactions had no coin-layer validation since they were added to NO_RECIPIENT_TX_TYPES (WCI-1111)
• Solution: Added specific validation in sol.ts:verifyTransaction (lines 574-614) that validates:
  • oldWithdrawAddress matches wallet root (preventing unauthorized authority changes)
  • newWithdrawAddress matches intended newWithdrawPublicKey from intent
  • stakingAddress matches intended stakeAccount from intent

2. Robust Parameter Validation ✅

// Validates critical fields when present
if (txParams.newWithdrawPublicKey && authorizeParams.newWithdrawAddress !== txParams.newWithdrawPublicKey) {
  throw new Error('StakingAuthorize newWithdrawAddress does not match intended newWithdrawPublicKey...');
}

• Prevents malicious substitution of withdraw authority
• Only validates when intent fields are provided (graceful degradation)
• Clear error messages for debugging

3. Secure Intent Parameter Propagation ✅

• Added newWithdrawPublicKey and stakeAccount to both PopulatedIntent and TransactionParams
• resolveEffectiveTxParams properly propagates these from intent to verification params
• Parameters flow correctly through the TSS signing pipeline

4. Enhanced explainTransaction Logic ✅

// Prioritizes Withdrawer instruction over Staker for stakingAuthorize summary
const isWithdrawerAuthorize = !!(
  authorizeInstruction.params.newWithdrawAddress || authorizeInstruction.params.custodianAddress
);

• Critical Fix: Now uses Withdrawer instruction fields (security-critical) instead of Staker
• Handles both WASM and non-WASM paths consistently
• Correctly identifies instruction type to populate the right fields

Attack Vector Analysis

✅ Malicious Server Attack - MITIGATED

• Scenario: Compromised BitGo server presents authorize tx that rotates withdraw authority to attacker key
• Protection: newWithdrawAddress validation prevents this - will throw if actual tx differs from intent

✅ Parameter Substitution - MITIGATED

• Scenario: Attacker swaps stakeAccount or newWithdrawPublicKey in transit
• Protection: Validation compares decoded instruction against original intent parameters

✅ Staker-Only Transaction Bypass - HANDLED

• Scenario: Legitimate staker authority changes shouldn't be blocked
• Protection: oldWithdrawAddress check only applies when field is non-empty (line 587-589)

Test Coverage Analysis ✅

The test suite comprehensively covers all security scenarios:

1. Happy Path: Valid authorize tx with matching parameters ✅
2. Optional Fields: Works when intent fields not provided ✅
3. Malicious newWithdrawPublicKey: Correctly rejects attacker substitution ✅
4. Wrong stakeAccount: Properly validates stake account matching ✅
5. Wrong oldWithdrawAddress: Ensures wallet root authority ✅
6. Fee Payer Validation: Doesn't bypass existing security checks ✅

Minor Observations

1. Code Quality ✅

• Clear, readable validation logic
• Appropriate error messages for debugging
• Follows existing patterns in the codebase

2. Backward Compatibility ✅

• Optional validation (only when intent fields present)
• No breaking changes to existing transaction flows
• Maintains support for staker-only transactions

3. Performance ✅

• Minimal overhead (simple field comparisons)
• No unnecessary computation or external calls

Potential Edge Cases (All Handled)

1. Empty oldWithdrawAddress: ✅ Correctly skipped for staker-only txs
2. Missing intent fields: ✅ Gracefully skips validation
3. Multiple authorize instructions: ✅ WASM path prioritizes Withdrawer instruction
4. Different transaction types: ✅ Only applies to actual authorize transactions

Conclusion

This implementation successfully closes the security gap identified in the Trail of Bits audit. The validation logic is robust, well-tested, and handles all identified attack vectors while maintaining backward compatibility.

Recommendation: APPROVE - This PR significantly improves the security posture of Solana staking authorize transactions without introducing regressions.

---

Job Run: Link
Branch: chalo-1294-sol-authorize-verify-transaction

[davidkaplanbitgo]

BTC related changes look fine