node-jose icon indicating copy to clipboard operation
node-jose copied to clipboard

Occasional corrupt private key, breaks when loaded with `asKey` with "RangeError: Attempt to access memory outside buffer bounds"

Open james-bowers opened this issue 2 years ago • 2 comments

I created this private key using node-jose, and then when I load it back into node jose using the asKey function, I get an error;

RangeError: Attempt to access memory outside buffer bounds
const privatePem = `-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIOeCAAAgHeSEY/fn0OYjJh49LKmVReGQmJcQx78u0sVCoAoGCCqGSM49
AwEHoUQDQgAEPwrrzWtZSsEb6EgldGFCnqbykydVPbvtP3H1UKs6EPDCl8mzGMHg
kksAw85JhXLAhy7b4WP0clQCXMDvJGanqg==
-----END EC PRIVATE KEY-----`;

await nodeJose.JWK.asKey(privatePem, 'pem');

This doesn't always happen, more often than not a private key is generated that works completely correctly.

The private key looks valid when I use the command

openssl ec -in broken-pem.pem -noout -text to validate it.

I'm on node v16.14.2 but i've tried it on a lot of different node versions using runkit, and using version 2.2.0 of node-jose.

I'm generating the keys using;

  const key = await jose.JWK.createKey('EC', 'P-256', { alg: '' });

  const privatePem = key.toPEM(true);
  const publicPem = key.toPEM(false);

which creates keys that work 99% of the time.

With the broken key

Screenshot 2023-04-19 at 18 16 04

With a valid key generated in the same way;

Screenshot 2023-04-19 at 18 16 49

This can happen for both public and private keys.

james-bowers avatar Apr 19 '23 17:04 james-bowers

Bitten by the same bug, test case;

        let iterations = 0
        try {
            for (let i = 0; i < 1000; ++i) {
                ++iterations
                const key = await jose.JWK.createKey('EC', 'P-256', { alg: 'ES256', use: 'sig' })
                const pem = key.toPEM(true)
                const key2 = await jose.JWK.asKey(pem, 'pem')
            }
        } catch (error) {
            fail(`Error after ${iterations} iterations`)
        }

wirde avatar May 26 '23 07:05 wirde

Also hit the same bug, and traced it through, notes.

  1. the fromDer call here defaults to decodeBitStrings: true: https://github.com/digitalbazaar/forge/blob/2bb97afb5058285ef09bcf1d04d6bd6b87cffd58/lib/asn1.js#L427-L434

  2. the EC point in these keys happens to be a valid asn1 object (containing, in this case, a bitstring), so gets decoded into a sub-object: https://github.com/digitalbazaar/forge/blob/2bb97afb5058285ef09bcf1d04d6bd6b87cffd58/lib/asn1.js#L590C16-L601

This happens, as I understand it, because the point starts with 04 3f, which is asn1 for "a 63-byte bitstring", and the point is 64 bytes of (close to) random noise?

  1. the sub-object arives at this Buffer.from call as an array: https://github.com/cisco/node-jose/blob/37db9e5371dc9a0557767a6a0190e2b0ab5cf3b6/lib/jwk/eckey.js#L323

  2. Buffer.from treats an array as if it was an array of integers, and objects are equal to zero, apparently:

> Buffer.from([{ value: 'potato' }], 'binary')
<Buffer 00 00>
  1. the buffer has a length of 1, so buffer#readUInt16BE throws, as 1 < 2.

FauxFaux avatar Jan 15 '24 19:01 FauxFaux