From 1f32e68c198c4db9bf3a28ca223ae131c768291d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 28 Jan 2026 13:52:11 +0000 Subject: [PATCH 1/8] Initial plan From df964d753a4e4d6b91427949c778e1a626625f2a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 28 Jan 2026 13:57:01 +0000 Subject: [PATCH 2/8] Fix wif.js corner case: validate array length before access Co-authored-by: ChALkeR <291301+ChALkeR@users.noreply.github.com> --- wif.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/wif.js b/wif.js index cc6737f0..690477ea 100644 --- a/wif.js +++ b/wif.js @@ -6,6 +6,7 @@ import { assertUint8 } from './assert.js' function from(arr, expectedVersion) { assertUint8(arr) + if (arr.length !== 33 && arr.length !== 34) throw new Error('Invalid WIF length') const version = arr[0] if (expectedVersion !== undefined && version !== expectedVersion) { throw new Error('Invalid network version') @@ -14,7 +15,6 @@ function from(arr, expectedVersion) { // Makes a copy, regardless of input being a Buffer or a Uint8Array (unlike .slice) const privateKey = Uint8Array.from(arr.subarray(1, 33)) if (arr.length === 33) return { version, privateKey, compressed: false } - if (arr.length !== 34) throw new Error('Invalid WIF length') if (arr[33] !== 1) throw new Error('Invalid compression flag') return { version, privateKey, compressed: true } } @@ -22,7 +22,6 @@ function from(arr, expectedVersion) { function to({ version: v, privateKey, compressed }) { if (!Number.isSafeInteger(v) || v < 0 || v > 0xff) throw new Error('Missing or invalid version') assertUint8(privateKey, { length: 32, name: 'privateKey' }) - if (privateKey.length !== 32) throw new TypeError('Invalid privateKey length') const out = new Uint8Array(compressed ? 34 : 33) out[0] = v out.set(privateKey, 1) From 51867693d496877c66d08900142e4c6cdf218cee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 28 Jan 2026 14:37:49 +0000 Subject: [PATCH 3/8] Add test case for invalid WIF array lengths Co-authored-by: ChALkeR <291301+ChALkeR@users.noreply.github.com> --- tests/wif.test.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/wif.test.js b/tests/wif.test.js index f4c1e94c..9ef3684e 100644 --- a/tests/wif.test.js +++ b/tests/wif.test.js @@ -73,3 +73,30 @@ test('sizes roundtrip, random data', async (t) => { } } }) + +test('invalid array lengths', async (t) => { + // Test that arrays with invalid lengths throw "Invalid WIF length" error + // Valid WIF arrays must be exactly 33 bytes (uncompressed) or 34 bytes (compressed) + const { toBase58checkSync } = await import('@exodus/bytes/base58check.js') + + // Test various invalid lengths + const invalidLengths = [0, 1, 4, 10, 32, 35, 50] + + for (const length of invalidLengths) { + const arr = new Uint8Array(length).fill(128) // Fill with non-zero to make it a valid base58check payload + const encoded = toBase58checkSync(arr) + + // Both sync and async versions should reject with "Invalid WIF length" + await t.assert.rejects( + async () => await lib.fromWifString(encoded), + { message: 'Invalid WIF length' }, + `fromWifString should reject length ${length}` + ) + + t.assert.throws( + () => lib.fromWifStringSync(encoded), + { message: 'Invalid WIF length' }, + `fromWifStringSync should reject length ${length}` + ) + } +}) From 48f70422b9f9e802864c15ffa32ff4b2a0711abd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 28 Jan 2026 14:52:48 +0000 Subject: [PATCH 4/8] Fix test to verify length validation happens before version check Co-authored-by: ChALkeR <291301+ChALkeR@users.noreply.github.com> --- tests/wif.test.js | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/tests/wif.test.js b/tests/wif.test.js index 9ef3684e..45a3996a 100644 --- a/tests/wif.test.js +++ b/tests/wif.test.js @@ -3,6 +3,7 @@ import { randomValues } from '@exodus/crypto/randomBytes' import { test } from 'node:test' import assert from 'node:assert/strict' import * as wif from 'wif' +import { toBase58checkSync } from '@exodus/bytes/base58check.js' async function fromWifString(str) { const a = await lib.fromWifString(str) @@ -74,29 +75,53 @@ test('sizes roundtrip, random data', async (t) => { } }) -test('invalid array lengths', async (t) => { +test('invalid array lengths throw before version check', async (t) => { // Test that arrays with invalid lengths throw "Invalid WIF length" error - // Valid WIF arrays must be exactly 33 bytes (uncompressed) or 34 bytes (compressed) - const { toBase58checkSync } = await import('@exodus/bytes/base58check.js') + // BEFORE checking the version, even when expectedVersion is provided. + // This is a regression test for the bounds check fix. + // + // Before fix: For invalid length arrays, arr[0] was read first, then: + // - If expectedVersion was provided and didn't match arr[0], threw "Invalid network version" + // - Otherwise, threw "Invalid WIF length" later + // After fix: Length validation happens first, always throwing "Invalid WIF length" + // for invalid lengths, regardless of expectedVersion // Test various invalid lengths const invalidLengths = [0, 1, 4, 10, 32, 35, 50] for (const length of invalidLengths) { - const arr = new Uint8Array(length).fill(128) // Fill with non-zero to make it a valid base58check payload + const arr = new Uint8Array(length).fill(128) + arr[0] = 42 // Set a specific version byte const encoded = toBase58checkSync(arr) - // Both sync and async versions should reject with "Invalid WIF length" + // Without expectedVersion: both old and new throw "Invalid WIF length" await t.assert.rejects( async () => await lib.fromWifString(encoded), { message: 'Invalid WIF length' }, - `fromWifString should reject length ${length}` + `fromWifString should reject length ${length} without expectedVersion` ) t.assert.throws( () => lib.fromWifStringSync(encoded), { message: 'Invalid WIF length' }, - `fromWifStringSync should reject length ${length}` + `fromWifStringSync should reject length ${length} without expectedVersion` + ) + + // With expectedVersion that doesn't match: + // - Old code would throw "Invalid network version" (after reading arr[0]) + // - New code throws "Invalid WIF length" (before reading arr[0]) + const wrongVersion = 99 // Different from arr[0] which is 42 + + await t.assert.rejects( + async () => await lib.fromWifString(encoded, wrongVersion), + { message: 'Invalid WIF length' }, + `fromWifString should reject length ${length} with wrong expectedVersion, throwing length error not version error` + ) + + t.assert.throws( + () => lib.fromWifStringSync(encoded, wrongVersion), + { message: 'Invalid WIF length' }, + `fromWifStringSync should reject length ${length} with wrong expectedVersion, throwing length error not version error` ) } }) From b723963b0dce6b0145a719d696813b899a7d5c11 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 28 Jan 2026 15:02:29 +0000 Subject: [PATCH 5/8] Simplify test and comments for invalid length validation Co-authored-by: ChALkeR <291301+ChALkeR@users.noreply.github.com> --- tests/wif.test.js | 42 ++++++++---------------------------------- 1 file changed, 8 insertions(+), 34 deletions(-) diff --git a/tests/wif.test.js b/tests/wif.test.js index 45a3996a..59d898e1 100644 --- a/tests/wif.test.js +++ b/tests/wif.test.js @@ -75,53 +75,27 @@ test('sizes roundtrip, random data', async (t) => { } }) -test('invalid array lengths throw before version check', async (t) => { - // Test that arrays with invalid lengths throw "Invalid WIF length" error - // BEFORE checking the version, even when expectedVersion is provided. - // This is a regression test for the bounds check fix. - // - // Before fix: For invalid length arrays, arr[0] was read first, then: - // - If expectedVersion was provided and didn't match arr[0], threw "Invalid network version" - // - Otherwise, threw "Invalid WIF length" later - // After fix: Length validation happens first, always throwing "Invalid WIF length" - // for invalid lengths, regardless of expectedVersion +test('invalid length throws before version check', async (t) => { + // Regression test: length validation before version check + // Old: threw "Invalid network version" for invalid length with wrong expectedVersion + // New: throws "Invalid WIF length" regardless of expectedVersion - // Test various invalid lengths const invalidLengths = [0, 1, 4, 10, 32, 35, 50] for (const length of invalidLengths) { const arr = new Uint8Array(length).fill(128) - arr[0] = 42 // Set a specific version byte + arr[0] = 42 const encoded = toBase58checkSync(arr) - - // Without expectedVersion: both old and new throw "Invalid WIF length" - await t.assert.rejects( - async () => await lib.fromWifString(encoded), - { message: 'Invalid WIF length' }, - `fromWifString should reject length ${length} without expectedVersion` - ) - - t.assert.throws( - () => lib.fromWifStringSync(encoded), - { message: 'Invalid WIF length' }, - `fromWifStringSync should reject length ${length} without expectedVersion` - ) - - // With expectedVersion that doesn't match: - // - Old code would throw "Invalid network version" (after reading arr[0]) - // - New code throws "Invalid WIF length" (before reading arr[0]) - const wrongVersion = 99 // Different from arr[0] which is 42 + const wrongVersion = 99 await t.assert.rejects( async () => await lib.fromWifString(encoded, wrongVersion), - { message: 'Invalid WIF length' }, - `fromWifString should reject length ${length} with wrong expectedVersion, throwing length error not version error` + { message: 'Invalid WIF length' } ) t.assert.throws( () => lib.fromWifStringSync(encoded, wrongVersion), - { message: 'Invalid WIF length' }, - `fromWifStringSync should reject length ${length} with wrong expectedVersion, throwing length error not version error` + { message: 'Invalid WIF length' } ) } }) From 7838a13cc6bf962b403fc9e094a9c890d3827885 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 28 Jan 2026 15:18:37 +0000 Subject: [PATCH 6/8] Move toBase58checkSync import to top of file Co-authored-by: ChALkeR <291301+ChALkeR@users.noreply.github.com> --- tests/wif.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/wif.test.js b/tests/wif.test.js index 59d898e1..a57ca939 100644 --- a/tests/wif.test.js +++ b/tests/wif.test.js @@ -1,9 +1,9 @@ import * as lib from '@exodus/bytes/wif.js' +import { toBase58checkSync } from '@exodus/bytes/base58check.js' import { randomValues } from '@exodus/crypto/randomBytes' import { test } from 'node:test' import assert from 'node:assert/strict' import * as wif from 'wif' -import { toBase58checkSync } from '@exodus/bytes/base58check.js' async function fromWifString(str) { const a = await lib.fromWifString(str) From d7a6b010ff9f1ce80aa2ed86525f4e9879a3b791 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 28 Jan 2026 15:29:53 +0000 Subject: [PATCH 7/8] Fix lint: rename loop variable from 'length' to 'len' Co-authored-by: ChALkeR <291301+ChALkeR@users.noreply.github.com> --- tests/wif.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/wif.test.js b/tests/wif.test.js index a57ca939..0dfab1e8 100644 --- a/tests/wif.test.js +++ b/tests/wif.test.js @@ -82,8 +82,8 @@ test('invalid length throws before version check', async (t) => { const invalidLengths = [0, 1, 4, 10, 32, 35, 50] - for (const length of invalidLengths) { - const arr = new Uint8Array(length).fill(128) + for (const len of invalidLengths) { + const arr = new Uint8Array(len).fill(128) arr[0] = 42 const encoded = toBase58checkSync(arr) const wrongVersion = 99 From 6044e62ea0134b0b184d16066f96c7d78767af94 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 28 Jan 2026 15:34:48 +0000 Subject: [PATCH 8/8] Fix prettier/lint formatting in test Co-authored-by: ChALkeR <291301+ChALkeR@users.noreply.github.com> --- tests/wif.test.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/wif.test.js b/tests/wif.test.js index 0dfab1e8..11d7c85c 100644 --- a/tests/wif.test.js +++ b/tests/wif.test.js @@ -79,23 +79,21 @@ test('invalid length throws before version check', async (t) => { // Regression test: length validation before version check // Old: threw "Invalid network version" for invalid length with wrong expectedVersion // New: throws "Invalid WIF length" regardless of expectedVersion - + const invalidLengths = [0, 1, 4, 10, 32, 35, 50] - + for (const len of invalidLengths) { const arr = new Uint8Array(len).fill(128) arr[0] = 42 const encoded = toBase58checkSync(arr) const wrongVersion = 99 - - await t.assert.rejects( - async () => await lib.fromWifString(encoded, wrongVersion), - { message: 'Invalid WIF length' } - ) - - t.assert.throws( - () => lib.fromWifStringSync(encoded, wrongVersion), - { message: 'Invalid WIF length' } - ) + + await t.assert.rejects(async () => lib.fromWifString(encoded, wrongVersion), { + message: 'Invalid WIF length', + }) + + t.assert.throws(() => lib.fromWifStringSync(encoded, wrongVersion), { + message: 'Invalid WIF length', + }) } })