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

A security issue in ECDH derive shared secret #3

Closed
shilohshi opened this issue Oct 24, 2022 · 4 comments
Closed

A security issue in ECDH derive shared secret #3

shilohshi opened this issue Oct 24, 2022 · 4 comments

Comments

@shilohshi
Copy link
Contributor

Hi.

Recently I found a vulnerability in

ecdh/index.js

Line 164 in 2aca0e3

PrivateKey.prototype.deriveSharedSecret = function(publicKey) {
, the deriveSharedSecret function only checks whether the format of the public key object is legal, but does not check whether its content is legal, this will lead to an invalid curve attack, an attacker can send an invalid point which not on the curve as public key, and then he can get the derived shared secret.

This is a classic attack method on ECDH, more details can be seen at https://crypto.stackexchange.com/questions/3820/why-do-public-keys-need-to-be-validated.

For example, if the attacker set the public key point to (0, 0), then the derived shared secret will always be 0:

// Change Bob's public key to a invalid point which not on the curve, e.g. (0, 0)
bobKeys.publicKey.Q.x.x = BigInteger.ZERO
bobKeys.publicKey.Q.y.x = BigInteger.ZERO

// Alice generate the shared secret:
var aliceSharedSecret = aliceKeys.privateKey.deriveSharedSecret(bobKeys.publicKey);

// the shared secret will always be 00000000000000000000000000000000
console.log('shared secret:', aliceSharedSecret.toString('hex'));

Since I see there are also some other projects depend on this implementation, I think it might be necessary to check and fix this vulnerability.

You can also contact me if you have any other question, best wishes.

@moshest
Copy link
Member

moshest commented Oct 24, 2022

Thanks @shilohshi , can you make a PR addressing it?

@shilohshi
Copy link
Contributor Author

Thanks @shilohshi , can you make a PR addressing it?

ok, I'll try to make a PR soon.

@shilohshi
Copy link
Contributor Author

A PR has been submitted.

@moshest
Copy link
Member

moshest commented Oct 24, 2022

Released under v0.2.0

@moshest moshest closed this as completed Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants