Skip to content

String deriviation paths cause invalid hardening value due to Lodash call #3358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SmartArray opened this issue Apr 27, 2022 · 2 comments
Closed
Labels

Comments

@SmartArray
Copy link

SmartArray commented Apr 27, 2022

We found a flaw with the string typed key derivation function at the following call:

HDPrivateKey.isValidPath = function(arg, hardened) {
if (_.isString(arg)) {
var indexes = HDPrivateKey._getDerivationIndexes(arg);
return indexes !== null && _.every(indexes, HDPrivateKey.isValidPath);
}

_.every calls the function HDPrivateKey.isValidPath with two args:

  1. The first arg is the expected parsed number of the derivation path
  2. The second (unexpected) arg is the index of the root array which toggles the hardening parameter of the function (isValidPath), which is clearly unintended

An easy fix is to introduce an anonymous function as shown in this PR: #3359

@kajoseph
Copy link
Collaborator

Thanks for reporting and the PR, @SmartArray. I am looking at this now

@kajoseph
Copy link
Collaborator

kajoseph commented May 11, 2022

@SmartArray Just so I'm clear, you are not having any issues with the outcome of isValidPath, correct? The strict check for hardened === true in the line below ensures that passing in the array index via _.every doesn't have an effect.

if (arg < HDPrivateKey.Hardened && hardened === true) {

If this is "just" a code smell issue (read: not imminently urgent), then we'll want to reconcile #3359 with #3350. The same issue exists, but will inevitably result in merge conflicts between the two.

@kajoseph kajoseph added the stale label Feb 11, 2025
@kajoseph kajoseph closed this as not planned Won't fix, can't repro, duplicate, stale Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants