Skip to content
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

Prevent [email protected] from mutating internal values #39

Merged
merged 1 commit into from
May 30, 2020

Conversation

alcuadrado
Copy link
Contributor

Hi,

We discovered a small bug in this library when integrating it into ethereumjs-wallet. You can read more about it here: ethereumjs/ethereumjs-wallet#127

The problem was that secp256k1 v4 mutates some values in place. v3 didn't. When upgrading this library, these method calls weren't updated so deriving a child key was sometimes modifying the private/public keys.

This PR fixes this bug, and adds some tests to prevent it from happening again.

Copy link
Member

@junderw junderw left a comment

Choose a reason for hiding this comment

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

This is pretty bad and needs a patch ASAP.

Thanks for the catch.

@junderw
Copy link
Member

junderw commented May 30, 2020

after publishing v2.0.1, I would consider depublishing v2.0.0 even...

@junderw
Copy link
Member

junderw commented May 30, 2020

@jprichardson @RyanZim

@alcuadrado
Copy link
Contributor Author

I think there's a small window of time to unpublish a version, which probably has closed. I think npm does exceptions if you contact their support channels though.

@RyanZim RyanZim merged commit 1b4bd4e into cryptocoinjs:master May 30, 2020
@RyanZim
Copy link
Member

RyanZim commented May 30, 2020

Published in 2.0.1

@RyanZim
Copy link
Member

RyanZim commented May 30, 2020

Deprecated hdkey v2.0.0, anyone attempting to install it (even as a sub-dependency) will get this logged in the console:

npm WARN deprecated [email protected]: Serious bug in hdkey v2.0.0; please use v2.0.1 or later. See https://github.com/cryptocoinjs/hdkey/pull/39 for details

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.

3 participants