txscript: handle variable length P2PKH signatures in ComputePkScript

Since P2PKH signatures have variable lengths, we would attempt to parse
P2PKH scripts as P2SH if they didn't fit the previous length
constraints.

btcec/signature.go

@@ -85,10 +85,10 @@ func (sig *Signature) IsEqual(otherSig *Signature) bool {
 		sig.S.Cmp(otherSig.S) == 0
 }
 
-// minSigLen is the minimum length of a DER encoded signature and is
-// when both R and S are 1 byte each.
+// MinSigLen is the minimum length of a DER encoded signature and is when both R
+// and S are 1 byte each.
 // 0x30 + <1-byte> + 0x02 + 0x01 + <byte> + 0x2 + 0x01 + <byte>
-const minSigLen = 8
+const MinSigLen = 8
 
 func parseSig(sigStr []byte, curve elliptic.Curve, der bool) (*Signature, error) {
 	// Originally this code used encoding/asn1 in order to parse the
@@ -103,7 +103,7 @@ func parseSig(sigStr []byte, curve elliptic.Curve, der bool) (*Signature, error)
 
 	signature := &Signature{}
 
-	if len(sigStr) < minSigLen {
+	if len(sigStr) < MinSigLen {
 		return nil, errors.New("malformed signature: too short")
 	}
 	// 0x30
@@ -118,7 +118,7 @@ func parseSig(sigStr []byte, curve elliptic.Curve, der bool) (*Signature, error)
 
 	// siglen should be less than the entire message and greater than
 	// the minimal message size.
-	if int(siglen+2) > len(sigStr) || int(siglen+2) < minSigLen {
+	if int(siglen+2) > len(sigStr) || int(siglen+2) < MinSigLen {
 		return nil, errors.New("malformed signature: bad length")
 	}
 	// trim the slice we're working on so we only look at what matters.

txscript/pkscript.go

@@ -13,13 +13,23 @@ import (
 )
 
 const (
-	// pubKeyHashSigScriptLen is the length of a signature script attempting
-	// to spend a P2PKH script. The only other possible length value is 107
-	// bytes, due to the signature within it. This length is determined by
-	// the following:
-	//   0x47 or 0x48 (71 or 72 byte data push) | <71 or 72 byte sig> |
-	//   0x21 (33 byte data push) | <33 byte compressed pubkey>
-	pubKeyHashSigScriptLen = 106
+	// minPubKeyHashSigScriptLen is the minimum length of a signature script
+	// that spends a P2PKH output. The length is composed of the following:
+	//   Signature length (1 byte)
+	//   Signature (min 8 bytes)
+	//   Signature hash type (1 byte)
+	//   Public key length (1 byte)
+	//   Public key (33 byte)
+	minPubKeyHashSigScriptLen = 1 + btcec.MinSigLen + 1 + 1 + 33
+
+	// maxPubKeyHashSigScriptLen is the maximum length of a signature script
+	// that spends a P2PKH output. The length is composed of the following:
+	//   Signature length (1 byte)
+	//   Signature (max 72 bytes)
+	//   Signature hash type (1 byte)
+	//   Public key length (1 byte)
+	//   Public key (33 byte)
+	maxPubKeyHashSigScriptLen = 1 + 72 + 1 + 1 + 33
 
 	// compressedPubKeyLen is the length in bytes of a compressed public
 	// key.
@@ -161,11 +171,19 @@ func ComputePkScript(sigScript []byte, witness wire.TxWitness) (PkScript, error)
 
 	// We'll start by checking the input's signature script, if provided.
 	switch {
+	case len(sigScript) == 0:
+		break
+
+	// Since we only support P2PKH and P2SH scripts as the only non-witness
+	// script types, we should expect to see a push only script.
+	case !IsPushOnlyScript(sigScript):
+		return pkScript, ErrUnsupportedScriptType
+
 	// If a signature script is provided with a length long enough to
 	// represent a P2PKH script, then we'll attempt to parse the compressed
 	// public key from it.
-	case len(sigScript) == pubKeyHashSigScriptLen ||
-		len(sigScript) == pubKeyHashSigScriptLen+1:
+	case len(sigScript) >= minPubKeyHashSigScriptLen &&
+		len(sigScript) <= maxPubKeyHashSigScriptLen:
 
 		// The public key should be found as the last part of the
 		// signature script. We'll attempt to parse it to ensure this is
@@ -183,14 +201,12 @@ func ComputePkScript(sigScript []byte, witness wire.TxWitness) (PkScript, error)
 			return pkScript, nil
 		}
 
-		// If it isn't, we'll assume it is a P2SH signature script.
 		fallthrough
 
 	// If we failed to parse a compressed public key from the script in the
-	// case above, or if the script length is not that of a P2PKH one, and
-	// our redeem script is only composed of data pushed, we can assume it's
-	// a P2SH signature script.
-	case len(sigScript) > 0 && IsPushOnlyScript(sigScript):
+	// case above, or if the script length is not that of a P2PKH one, we
+	// can assume it's a P2SH signature script.
+	default:
 		// The redeem script will always be the last data push of the
 		// signature script, so we'll parse the script into opcodes to
 		// obtain it.
@@ -209,9 +225,6 @@ func ComputePkScript(sigScript []byte, witness wire.TxWitness) (PkScript, error)
 		pkScript.class = ScriptHashTy
 		copy(pkScript.script[:], script)
 		return pkScript, nil
-
-	case len(sigScript) > 0:
-		return pkScript, ErrUnsupportedScriptType
 	}
 
 	// If a witness was provided instead, we'll use the last item of the

txscript/pkscript_test.go

@@ -208,9 +208,9 @@ func TestComputePkScript(t *testing.T) {
 		{
 			name: "P2PKH sigScript",
 			sigScript: []byte{
-				// OP_DATA_71,
-				0x47,
-				// <71-byte sig>
+				// OP_DATA_73,
+				0x49,
+				// <73-byte sig>
 				0x30, 0x44, 0x02, 0x20, 0x65, 0x92, 0xd8, 0x8e,
 				0x1d, 0x0a, 0x4a, 0x3c, 0xc5, 0x9f, 0x92, 0xae,
 				0xfe, 0x62, 0x54, 0x74, 0xa9, 0x4d, 0x13, 0xa5,
@@ -219,7 +219,8 @@ func TestComputePkScript(t *testing.T) {
 				0x36, 0x96, 0x19, 0x1f, 0xb7, 0x00, 0xc5, 0xa7,
 				0x7e, 0x22, 0xd9, 0xfb, 0x6b, 0x42, 0x67, 0x42,
 				0xa4, 0x2c, 0xac, 0xdb, 0x74, 0xa2, 0x7c, 0x43,
-				0xcd, 0x89, 0xa0, 0xf9, 0x44, 0x54, 0x01,
+				0xcd, 0x89, 0xa0, 0xf9, 0x44, 0x54, 0x12, 0x74,
+				0x01,
 				// OP_DATA_33
 				0x21,
 				// <33-byte compressed pubkey>