Skip to content

[GHSA-fc9h-whq2-v747] Valid ECDSA signatures erroneously rejected in Elliptic #5442

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

Open
wants to merge 1 commit into
base: tal-sealsecurity/advisory-improvement-5442
Choose a base branch
from

Conversation

tal-sealsecurity
Copy link

Updates

  • Affected products
  • CVSS v3

Comments
More edge cases: k hash is generated with leading 0 and given to BN in this example).
Continued discussion on indutny/elliptic#321
indutny/elliptic#321 (comment)
indutny/elliptic#321 (comment)

@Copilot Copilot AI review requested due to automatic review settings April 7, 2025 09:32
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)

@github-actions github-actions bot changed the base branch from main to tal-sealsecurity/advisory-improvement-5442 April 7, 2025 09:33
@shelbyc
Copy link
Contributor

shelbyc commented Apr 9, 2025

Hi @tal-sealsecurity, I read indutny/elliptic#321 (comment) and indutny/elliptic#321 (comment) and showed them to my colleagues. If the comments describe a bypass of the fix for CVE-2024-48948, as indutny/elliptic#321 (comment) suggests, getting a new CVE for the bypass is an option but not a requirement. If @indutny wants a CVE and makes a repository GHSA, GitHub can issue such a CVE. GitHub's CNA scope limits CVE issuing to maintainers using the repo GHSA feature.

However, and I'm not a cryptography expert by any means, the comments read to me like they might be describing a separate vulnerability? I would be curious to hear more from @indutny and @bleichenbacher-daniel about the relationship between CVE-2024-48948 and the issues described in indutny/elliptic#321 (comment) and indutny/elliptic#321 (comment).

@bleichenbacher-daniel
Copy link

It is essentially a deeper analysis of a bug in the library. Sometimes when one finds a bug in a library then it looks like this bug is benign, but further analysis reveals that the bug actually can be exploited. This happened here. The first reaction was that the bug is not critical. A more careful analysis however, shows that in some situations the private key can be leaked. For this to happen the attacker would need a faulty signature and also the correct signature for the same message.
What would speak for a separate security advisory/CVE/... is the observation that a key revocation might be necessary. If someone generated faulty signatures for a message then they try to avoid generating the correct signature for the same message with the same key again, since this now allows an attacker to find the private key.

@tal-sealsecurity
Copy link
Author

tal-sealsecurity commented Apr 10, 2025

Thanks @shelbyc and @bleichenbacher-daniel !
The leading zero might also be an issue for private key generation.
Posting it here since this can lead to an invalid private key / signature which might be related to the CVE.

This is due to private key with lower byte length.
source code: var priv = new BN(drbg.generate(bytes));
When HmacDRBG is generating a byte array that can contain a leading zero.
BN then take and omit this leading zero, causing a byte loss.

This will cause an invalid signature / private key when tested on other apps
Tested with 3rd party apps, pass on elliptic though:

  • python ecdsa, failed on invalid private key
  • python cryptography, failed on invalid signature

code to generate a low byte key:

const EC = require('elliptic').ec;
const ec = new EC('p521');

for (;;) {
  const _key = ec.genKeyPair();
  const _privateKeyHex = _key.getPrivate('hex');
  if (_privateKeyHex && _key.getPrivate('hex').length / 2 != 66) {
    console.log("The private key bytes:", _key.getPrivate('hex').length / 2);
  }
}

Example output:


Public Key (hex): 0400089742d2937064fe6ff4977d6acdcd2f36b21a428ca126622e863adf41fc6f88382e0a7cd1e458ba57a80c6ff84261ebb3452338f0eac01f1f6ff75a40ca1a12c3012b1951d0907beb185b370942baac641fc761019764d834d7162e3b5b735270d67f861bb572422ca4674f1b91d0084b76bb63233ceb86d5fbf66a9bc7f4667482eb

Private Key (hex): 42897ebf617ae095f9919ffd91c042e6af887ec04da609cded31c2e16d84eb253877a3fa4639b6b94d18e9b6586d879d3ba5c6f4eb7b94a05e0eeca40cbd31eb24

Message (hex): “12f830e9591916ec"

Signature (DER hex): 308188024201d483a76a185669a3c9a31be99706ccd806d58cd8320b8b074ca24a79cd32b13c6b84bbd53532a1147e8c4f6b082e0ef620de9f74e5eaf8cba1aff2bab6a871d8da024200a0e5eff83bf735f42261dc83f9d358c7cadd8a8406f2bb4d71602f9449c208b0dcabab60d7ecc843f1b553a89f8e7e7c55d802b0a6f033625b182da59168ad7402

Sample file:

const { createHash } = require('crypto');
const EC = require('elliptic').ec;

// Initialize the EC context for secp521r1
const ec = new EC('p521');

const PRIV = "42897ebf617ae095f9919ffd91c042e6af887ec04da609cded31c2e16d84eb253877a3fa4639b6b94d18e9b6586d879d3ba5c6f4eb7b94a05e0eeca40cbd31eb24"
const msg = "12f830e9591916ec";
const bytes = Buffer.from(msg, 'hex');
function sha512ByteArray(message) {
    const hash = createHash('sha512');
    hash.update(message);
    return hash.digest(); // Returns a Buffer, which is very close to a byte array
  }
const msgHash = sha512ByteArray(bytes);

// Generate a new key pair
const key = ec.keyFromPrivate(PRIV, 'hex');
console.log('Public Key (hex):', key.getPublic('hex'));
console.log('Private Key (hex):', key.getPrivate('hex')); 
console.log('Private Key (bytes):', key.getPrivate('hex').length / 2); // expected 66, got 65

// Sign the hashed message
const signature = key.sign(msgHash);
console.log('Signature (DER hex):', signature.toDER('hex'));
const valid = key.verify(msgHash, signature);
if (!valid) {
    console.error('❌ Signature verification failed!');
}

@bleichenbacher-daniel
Copy link

Just a quick note. I may have some time next week to look into this.

I haven't looked into private key generation, so I can't comment on this one.

The libraries that pass all my tests for ECDSA over prime curves with RFC 6979 are:

  • pyca (python cryptography)
  • BouncyCastle
    and
  • pycryptodome.
    For pyca and BouncyCastle I'm using DER encoded signatures, for pycryptodome I'm using P1363 encoded signatures.
    None of the libraries normalize their signatures.

I would think that comparing against any of these libraries gives a reliable comparison.
I haven't tested python ecdsa, so I can't comment on this either.

The test vectors I'm using are here: https://github.com/bleichenbacher-daniel/Rooterberg

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