Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

txscript: Tighten standardness pubkey checks. #1649

Merged
merged 1 commit into from Mar 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 15 additions & 6 deletions txscript/engine.go
Expand Up @@ -400,18 +400,27 @@ func (vm *Engine) checkHashTypeEncoding(hashType SigHashType) error {
return nil
}

// checkPubKeyEncoding returns whether or not the passed public key adheres to
// the strict encoding requirements.
func (vm *Engine) checkPubKeyEncoding(pubKey []byte) error {
// isStrictPubKeyEncoding returns whether or not the passed public key adheres
// to the strict encoding requirements.
func isStrictPubKeyEncoding(pubKey []byte) bool {
if len(pubKey) == 33 && (pubKey[0] == 0x02 || pubKey[0] == 0x03) {
// Compressed
return nil
return true
}
if len(pubKey) == 65 && pubKey[0] == 0x04 {
// Uncompressed
return nil
return true
}
return scriptError(ErrPubKeyType, "unsupported public key type")
return false
}

// checkPubKeyEncoding returns an error if the passed public key does not
// adhere to the strict encoding requirements.
func (vm *Engine) checkPubKeyEncoding(pubKey []byte) error {
if !isStrictPubKeyEncoding(pubKey) {
return scriptError(ErrPubKeyType, "unsupported public key type")
}
return nil
}

// checkSignatureEncoding returns whether or not the passed signature adheres to
Expand Down
10 changes: 6 additions & 4 deletions txscript/standard.go
Expand Up @@ -76,9 +76,10 @@ func (t ScriptClass) String() string {
// isPubkey returns true if the script passed is a pay-to-pubkey transaction,
// false otherwise.
func isPubkey(pops []parsedOpcode) bool {
// Valid pubkeys are either 33 or 65 bytes.
// Valid pubkeys are either 33 or 65 bytes and must start with the correct
// prefix for the given length.
return len(pops) == 2 &&
(len(pops[0].data) == 33 || len(pops[0].data) == 65) &&
isStrictPubKeyEncoding(pops[0].data) &&
pops[1].opcode.value == OP_CHECKSIG
}

Expand Down Expand Up @@ -163,8 +164,9 @@ func isMultiSig(pops []parsedOpcode) bool {
}

for _, pop := range pops[1 : l-2] {
// Valid pubkeys are either 33 or 65 bytes.
if len(pop.data) != 33 && len(pop.data) != 65 {
// Valid pubkeys are either 33 or 65 bytes and must start with
// the correct prefix for the given length.
if !isStrictPubKeyEncoding(pop.data) {
return false
}
}
Expand Down
31 changes: 16 additions & 15 deletions txscript/standard_test.go
Expand Up @@ -268,35 +268,37 @@ func TestExtractPkScriptAddrs(t *testing.T) {
reqSigs: 0,
class: NonStandardTy,
},
// from real tx 691dd277dc0e90a462a3d652a1171686de49cf19067cd33c7df0392833fb986a, vout 0
// adapted from btc:
// tx 691dd277dc0e90a462a3d652a1171686de49cf19067cd33c7df0392833fb986a, vout 0
// invalid public keys
{
name: "1 of 3 multisig with invalid pubkeys",
script: hexToBytes("51411c2200007353455857696b696c656" +
script: hexToBytes("5141042200007353455857696b696c656" +
"16b73204361626c6567617465204261636b75700a0a6" +
"361626c65676174652d3230313031323034313831312" +
"e377a0a0a446f41776e6c6f61642074686520666f6c6" +
"e377a0a0a446f41046e6c6f61642074686520666f6c6" +
"c6f77696e67207472616e73616374696f6e732077697" +
"468205361746f736869204e616b616d6f746f2773206" +
"46f776e6c6f61416420746f6f6c2077686963680a636" +
"46f776e6c6f61410420746f6f6c2077686963680a636" +
"16e20626520666f756e6420696e207472616e7361637" +
"4696f6e2036633533636439383731313965663739376" +
"435616463636453ae"),
addrs: []dcrutil.Address{},
reqSigs: 1,
class: MultiSigTy,
},
// from real tx: 691dd277dc0e90a462a3d652a1171686de49cf19067cd33c7df0392833fb986a, vout 44
// adapted from btc:
// tx 691dd277dc0e90a462a3d652a1171686de49cf19067cd33c7df0392833fb986a, vout 44
// invalid public keys
{
name: "1 of 3 multisig with invalid pubkeys 2",
script: hexToBytes("514134633365633235396337346461636" +
script: hexToBytes("514104633365633235396337346461636" +
"536666430383862343463656638630a6336366263313" +
"93936633862393461333831316233363536313866653" +
"16539623162354136636163636539393361333938386" +
"16539623162354104636163636539393361333938386" +
"134363966636336643664616266640a3236363363666" +
"13963663463303363363039633539336333653931666" +
"56465373032392131323364643432643235363339643" +
"56465373032392102323364643432643235363339643" +
"338613663663530616234636434340a00000053ae"),
addrs: []dcrutil.Address{},
reqSigs: 1,
Expand Down Expand Up @@ -420,20 +422,19 @@ func TestCalcScriptInfo(t *testing.T) {
},
{
// Script is invented, numbers all fake.
name: "multisig script",
// Extra 0 arg on the end for OP_CHECKMULTISIG bug.
sigScript: "1 1 1 0",
name: "multisig script",
sigScript: "1 1 1",
pkScript: "3 " +
"DATA_33 0x0102030405060708090a0b0c0d0e0f1011" +
"DATA_33 0x0202030405060708090a0b0c0d0e0f1011" +
"12131415161718191a1b1c1d1e1f2021 DATA_33 " +
"0x0102030405060708090a0b0c0d0e0f101112131415" +
"161718191a1b1c1d1e1f2021 DATA_33 0x010203040" +
"0x0302030405060708090a0b0c0d0e0f101112131415" +
"161718191a1b1c1d1e1f2021 DATA_33 0x020203040" +
"5060708090a0b0c0d0e0f101112131415161718191a1" +
"b1c1d1e1f2021 3 CHECKMULTISIG",
bip16: true,
scriptInfo: ScriptInfo{
PkScriptClass: MultiSigTy,
NumInputs: 4,
NumInputs: 3,
ExpectedInputs: 3,
SigOps: 3,
},
Expand Down