secretkeylabs / xverse-core

Core library for Xverse wallet
Other
38 stars 19 forks source link

false positive result for Taproot message signature verification #223

Closed iamcrazycoder closed 1 year ago

iamcrazycoder commented 1 year ago

I have generated a message signature by signing w/ a Taproot address using the XVerse extension. While the verification works fine for p2sh signatures, it returns false positives for p2tr signatures.

This issue can be reproduced using the following code and by updating the 2nd argument value ("Hello") of verifySignature to any other value. The result is always true for any message value.

[referenced from connect/bip322Signature.ts]

import * as secp256k1 from '@noble/secp256k1'; // install v1.7.1
import { base64 } from '@scure/base';
import { AddressType, getAddressInfo } from 'bitcoin-address-validation';
import { crypto } from 'bitcoinjs-lib';
import { verify } from 'bitcoinjs-message';

const verifySignature = (address, message, signature) => {
  if (!address) throw new Error('Invalid Address');
  const { type } = getAddressInfo(address);
  if (type === AddressType.p2sh) {
    return verify(message, address, base64.decode(signature).slice(1));
  }
  const decodedSignature = base64.decode(signature).slice(2);
  if (!decodedSignature) throw new Error('Malformed Signature');
  const msgHash = bip0322Hash(message);
  const pk = secp256k1.recoverPublicKey(msgHash, decodedSignature, 1, true);
  const isValid = secp256k1.verify(decodedSignature, msgHash, pk, { strict: true });
  return isValid;
}

function bip0322Hash(message) {
  const { sha256 } = crypto;
  const tag = 'BIP0322-signed-message';
  const tagHash = sha256(Buffer.from(tag));
  const result = sha256(Buffer.concat([tagHash, tagHash, Buffer.from(message)]));
  return result.toString('hex');
}

function verify() {
    const signature = 'AUD3T2beljO9JyTWpqB5YiqR3HlqytBLkxQL3z1C6MLe3lxzFes1RPwZngsBoFGfErNnmujiGS3Ua9GJC+YhQrHS'
    const result = verifySignature('tb1phz0777slw09a02vjud8k795nupww3gvhr9ax6nxuw9g5ayun5pqsnce05m', 'Hello', signature)

    console.log({ result })
}

verify()
kodemon commented 1 year ago

https://github.com/secretkeylabs/xverse-core/blob/develop/tests/singatures/bip32.test.ts#L18

Replacing message variable here with any value that isn't the original message that was signed returns true where it should return false.

victorkirov commented 1 year ago

Hi @iamcrazycoder @kodemon. Apologies for the late reply.

The verify function was only used to validate that the resultant signature, matched the address. It was placed in the wrong place as it was for testing only, and it didn't actually validate the bip-322 message in the payload.

We've created a ticket to clean this up a little and add the required documentation. To validate the actual message/address combo in the signature, you can use a library like bip322-js: https://github.com/ACken2/bip322-js