search

cryptocoinjs / secp256k1-node

Node.js binding for an Optimized C library for EC operations on curve secp256k1
Other
341 stars 120 forks source link

Different behavior between Node and Browser for invalid SEC1 key #166

Closed cgewecke closed 4 years ago

cgewecke commented 4 years ago

Hi, ethereumjs-util PR 228 is upgrading secp256k1 from ^3.0.1 to ^4.0.0 and seeing a couple tests for invalid SEC1 keys fail in browser (karma) tests, but pass ok in Node 10. The tests worked in both contexts using 3.0.x

For example, one test case takes an invalid SEC1 key:

const key = Buffer.from('023a443d8381a6798a70c6ff9304bdc8cb0163c23211d11628fae52ef9e0dca11a001cf066d56a8156fc201cd5df8a36ef694eecd258903fca7086c1fae7441e1d',
      'hex',
    )

and passes it to publicKeyVerify expecting it to fail. 

secp256k1.publicKeyVerify(key) // Returns false in Node, true in Chrome/Firefox

Do you have any advice about this? Is it happening because elliptic treats this key as compressed and as such its valid?

fanatid commented 4 years ago

Thanks for issue, I checked code and problem is that bitcoin-core/secp256k1 check input length, while elliptic fallback do not check length for key type. Will push fix shortly.

fanatid commented 4 years ago

Probably commented lines for debug on PR #160, but not uncomment back =( Before PR: https://github.com/cryptocoinjs/secp256k1-node/pull/160/files#diff-90efa0327a25e1361ca8efd38b80c8f4L46-L61 After PR: https://github.com/cryptocoinjs/secp256k1-node/pull/160/files#diff-aa0b70ce0bf24e5c6f1f19d26feec16bR44-R60

fanatid commented 4 years ago

Fix published in 4.0.1