DER parser failing to parse r and s values about elliptic HOT 6 CLOSED

The DER encoding on that signature is wrong, its 1 byte longer than it says it is (expected 71, got 73).

from elliptic.

@dcousens, yep, just noticed this. Elliptic looks like it does strict length checking on the elements in the signatures and the signature itself. I wrote a function to attempt to shorten overly long DER signatures before passing them into elliptic (in case anyone else runs into this): https://gist.github.com/chjj/1d26c818f2e20afcc33d

Closing now.

from elliptic.

@chjj this is very important to reject such signatures. If you allow multiplicity in signature encoding - there are potential attacks on hash collisions. I really advise you against using it for new signatures, it can be used to verify old ones (up to some minimal point in the past).

from elliptic.

@indutny, eventually the bitcoin DERSIG flag will be activated by bcoin once the blocks reach a certain version threshold, requiring signatures to follow the standard encoding, so this shouldn't be an issue. All transactions accepted to the mempool already get checked for strict DER signature encoding before being verified.

I'm technically only validating historical data here which isn't usually done due to bitcoin's checkpoint system.

See: https://github.com/indutny/bcoin/blob/fullnode/lib/bcoin/script.js#L2187

So, yes, disclaimer to anyone who needs to use my example above: make sure you verify the standard DER encoding yourself for non-historical data.

from elliptic.

@chjj Fantastic! I just wanted to be sure that there is no security risk for you guys!

from elliptic.

@indutny, thank you. :)

I added an extra note in the comments and a failsafe to make sure this is never done for non-historical data.

from elliptic.