This repository has been archived by the owner on Apr 3, 2019. It is now read-only.
Don't mutate the elliptic curve prototype with get* additions #238
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Point's prototype is set to the prototype of ec('secp256k1').curve.point(), which means mutation to it is shared across users of 'elliptic' via the in-memory representation of those objects.
This is not generally a problem, for example, with `validate` it's a simple set, so the greatest risk is that it will be directly overwritten.
But in the case of `getX` and `getY`, both methods are overwritten with others that depend on the prior implementation as stored in `_get*`. If this happens twice, then the implementation of `_getX` is replaced with something that depends on a call to `_getX`, and the callers is stuck in an infinite loop.
|
I would like to also remove the |
|
I also tried adding an intermediate object into the prototype chain, by wrapping |
|
Hi, thanks a lot for the insight. I think I have hit this problem before when bundling bitcore-lib and bitcore-lib-cash on the same JS using webpack. we will review the PR soon. Thanks again! |
|
Greate improvement, thank you, @Empact! Currently using of |
|
I think the same should also apply to bn.js I hope all other forks merge this one rapidly. |
This was referenced Oct 19, 2018
|
Yeah, ideally we should make upstream libraries completely isolated / independent. Could do that by using composition instead of inheritance. |
|
Yes,
Definitely agree. These implementations should be done in an isolated
module where it exposes the same methods from main libraries and also
provides its own new methods, something like a wrapper.
The only drawback with the mentioned solution is that a lot of places in
bitcore-lib should change to use new module instead of main ec and bn.
…On Sat, 20 Oct 2018, 03:42 Ben Woosley, ***@***.***> wrote:
Yeah, ideally we should make upstream libraries completely isolated /
independent. Could do that by using composition instead of inheritance.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#238 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGbOZEV8JAJhYFBWdl6K8Dt2MdJ5145Gks5ummp7gaJpZM4TwgQq>
.
|
baryon
added a commit
to baryon/bsv
that referenced
this pull request
Jan 12, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Point's prototype is set to the prototype of
ec('secp256k1').curve.point(), which means the library itself is global state, and mutation to it is shared across users of 'elliptic'.This is not generally a problem, for example, with
validateit's a simple set, so the greatest risk is that it will be directly overwritten.But in the case of
getXandgetY, both methods are overwritten with others that depend on the prior implementation as stored in_get*. If this happens twice, then the implementation of_getXis replaced with something that depends on a call to_getX, and the callers is stuck in an infinite loop.Here's an example of the infinite loop, which I came across by interaction between bitcore-lib-dash and zcash-bitcore-lib, but by inspection it seems they are both downstream of this code.