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

[tal-sealsecurity]

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 wasn't able to review any files in this pull request.

Files not reviewed (1)

• advisories/github-reviewed/2024/10/GHSA-fc9h-whq2-v747/GHSA-fc9h-whq2-v747.json: Language not supported

[shelbyc]

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]

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]

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]

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

[github-actions]

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the Keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot. Please see CONTRIBUTING.md for more policy details.

[bleichenbacher-daniel]

Just one small comment. I looked at the example from #5442 (comment) :

Public Key (hex): 0400089742d2937064fe6ff4977d6acdcd2f36b21a428ca126622e863adf41fc6f88382e0a7cd1e458ba57a80c6ff84261ebb3452338f0eac01f1f6ff75a40ca1a12c3012b1951d0907beb185b370942baac641fc761019764d834d7162e3b5b735270d67f861bb572422ca4674f1b91d0084b76bb63233ceb86d5fbf66a9bc7f4667482eb

Private Key (hex): 42897ebf617ae095f9919ffd91c042e6af887ec04da609cded31c2e16d84eb253877a3fa4639b6b94d18e9b6586d879d3ba5c6f4eb7b94a05e0eeca40cbd31eb24

Message (hex): “12f830e9591916ec"

Signature (DER hex): 308188024201d483a76a185669a3c9a31be99706ccd806d58cd8320b8b074ca24a79cd32b13c6b84bbd53532a1147e8c4f6b082e0ef620de9f74e5eaf8cba1aff2bab6a871d8da024200a0e5eff83bf735f42261dc83f9d358c7cadd8a8406f2bb4d71602f9449c208b0dcabab60d7ecc843f1b553a89f8e7e7c55d802b0a6f033625b182da59168ad7402

While the private key is indeed too short, the signature itself seems correct. I.e., I get the same signature with multiple libraries when a zero byte is prepended to the private key. Interestingly, this also includes python cryptography (pyca). Python cryptography can be built against different underlying libraries. Not sure if this could cause a discrepancy.

I've also been running tests with more extreme edge cases for the private key with no new results.