From c3591d88976b828ec76108e925e61b78fbb48ab0 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 30 Sep 2019 16:52:35 +0300 Subject: [PATCH] vm: fix CHECKMULTISIG to comply with NEO VM implementation Testing with testnet quickly revealed that our code has an issue when the key count doesn't equal signature count, fix it and add some comments. --- pkg/vm/vm.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index a3ad75215..49f20259b 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -907,6 +907,8 @@ func (v *VM) execute(ctx *Context, op Instruction) { if err != nil { panic(fmt.Sprintf("wrong parameters: %s", err.Error())) } + // It's ok to have more keys than there are signatures (it would + // just mean that some keys didn't sign), but not the other way around. if len(pkeys) < len(sigs) { panic("more signatures than there are keys") } @@ -914,18 +916,24 @@ func (v *VM) execute(ctx *Context, op Instruction) { panic("VM is not set up properly for signature checks") } sigok := true + // j counts keys and i counts signatures. j := 0 - for i := 0; sigok && i < len(pkeys) && j < len(sigs); { + for i := 0; sigok && j < len(pkeys) && i < len(sigs); { pkey := &keys.PublicKey{} err := pkey.DecodeBytes(pkeys[j]) if err != nil { panic(err.Error()) } + // We only move to the next signature if the check was + // successful, but if it's not maybe the next key will + // fit, so we always move to the next key. if pkey.Verify(sigs[i], v.checkhash) { i++ } j++ - if len(pkeys)-i > len(sigs)-j { + // When there are more signatures left to check than + // there are keys the check won't successed for sure. + if len(sigs)-i > len(pkeys)-j { sigok = false } }