Skip to content

fix(abstract-utxo): reject invalid chain and index values - #9401

Open
Daan-25 wants to merge 1 commit into
BitGo:masterfrom
Daan-25:fix/fixed-script-invalid-index
Open

fix(abstract-utxo): reject invalid chain and index values#9401
Daan-25 wants to merge 1 commit into
BitGo:masterfrom
Daan-25:fix/fixed-script-invalid-index

Conversation

@Daan-25

@Daan-25 Daan-25 commented Aug 2, 2026

Copy link
Copy Markdown

Description

Reject invalid fixed-script derivation chain and index values instead of silently normalizing them to default values.

Previously:

  • negative and non-integer indexes could be treated as index 0;
  • non-integer chain values could fall back to the default chain.

This could allow invalid derivation metadata to validate against an address derived with different parameters.

This change requires explicitly supplied chain and index values to be valid integers. Indexes must also be non-negative.

Tests

Added regression coverage confirming that:

  • negative indexes are rejected during address generation and validation;
  • non-integer indexes are rejected during address generation and validation;
  • non-integer chain values are rejected during address generation and validation;
  • index 0 remains valid.

Validation

  • git diff --check
  • yarn workspace @bitgo/abstract-utxo check-fmt
  • yarn workspace @bitgo/abstract-utxo lint
  • yarn workspace @bitgo/abstract-utxo build
  • yarn workspace @bitgo/abstract-utxo mocha test/unit/address-index-edge.test.ts — 7 passing

@Daan-25
Daan-25 marked this pull request as ready for review August 2, 2026 22:56
@Daan-25
Daan-25 requested a review from a team as a code owner August 2, 2026 22:56
Copilot AI review requested due to automatic review settings August 2, 2026 22:56
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens fixed-script (2-of-3 multisig) address derivation handling in @bitgo/abstract-utxo by rejecting invalid derivation indexes (negative or non-integer) instead of silently normalizing them to 0, and adds regression tests to prevent reintroduction of the issue.

Changes:

  • Enforce index to be a non-negative integer in generateAddress.
  • Enforce index to be a non-negative integer in assertFixedScriptWalletAddress.
  • Add unit coverage for invalid/valid index edge cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
modules/abstract-utxo/src/address/fixedScript.ts Adds stricter validation for derivation index during generation and validation.
modules/abstract-utxo/test/unit/address-index-edge.test.ts Adds regression tests ensuring invalid indexes are rejected and index 0 remains valid.
Suppressed comments (1)

modules/abstract-utxo/src/address/fixedScript.ts:169

  • assertFixedScriptWalletAddress still allows non-integer chain values to pass the initial validation (_.isFinite(chain)), and generateAddress will then ignore such values and fall back to the default chain. To prevent validating an address against mismatched derivation metadata, require chain to be an integer as well.
  if (
    (_.isUndefined(chain) && _.isUndefined(index)) ||
    !(_.isFinite(chain) && _.isFinite(index)) ||
    !_.isInteger(index) ||
    (index as number) < 0
  ) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modules/abstract-utxo/src/address/fixedScript.ts
Daan-25 added a commit to Daan-25/BitGoJS that referenced this pull request Aug 2, 2026
@Daan-25
Daan-25 force-pushed the fix/fixed-script-invalid-index branch from c8ab32e to 461f61c Compare August 2, 2026 23:18
Daan-25 added a commit to Daan-25/BitGoJS that referenced this pull request Aug 2, 2026
@Daan-25
Daan-25 force-pushed the fix/fixed-script-invalid-index branch from 461f61c to 7e9f618 Compare August 2, 2026 23:22
@Daan-25 Daan-25 changed the title fix(abstract-utxo): reject invalid derivation indexes fix(abstract-utxo): reject invalid chain and index values Aug 2, 2026
@Daan-25
Daan-25 force-pushed the fix/fixed-script-invalid-index branch from 7e9f618 to 38a13e2 Compare August 2, 2026 23:28
@Daan-25

Daan-25 commented Aug 2, 2026

Copy link
Copy Markdown
Author

Hi BitGo team, this is an external contribution. The commit is GPG-signed and GitHub reports the signature as verified and valid, but the Validate Humans In The Loop check reports that my key is not in BitGo's internal engineer allowlist. Could a maintainer advise on the expected process for external contributors, approve the pending workflows, and review the change? The requested chain validation update has also been implemented, and the targeted tests, formatting, lint, and build all pass locally.

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.

2 participants