From b483914d9d6ed6c3c28c7ba3dcfa377e4b1e1a0b Mon Sep 17 00:00:00 2001
From: "Jeff R. Allen" <jeff.allen@epfl.ch>
Date: Wed, 20 Jun 2018 16:38:54 +0200
Subject: [PATCH] Do not allow verifier panic to take down the server

If a verifier panics, do not crash. Instead report the error
and treat the verifier as if it failed.
---
 skipchain/skipchain.go      | 15 ++++++++++++++-
 skipchain/skipchain_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/skipchain/skipchain.go b/skipchain/skipchain.go
index e0fea0ba8e..31b7e16b75 100644
--- a/skipchain/skipchain.go
+++ b/skipchain/skipchain.go
@@ -943,7 +943,20 @@ func (s *Service) bftForwardLinkLevel0(msg, data []byte) bool {
 				log.Lvlf2("Found no user verification for %x", ver)
 				return false
 			}
-			if !f(fl.To, fs.Newest) {
+			// Now we call the verification function. Wrap up f() inside of
+			// g(), so that we can recover panics from f().
+			g := func(to []byte, newest *SkipBlock) (out bool) {
+				defer func() {
+					if re := recover(); re != nil {
+						log.Error("verification function panic: " + re.(string))
+						out = false
+					}
+				}()
+				out = f(to, newest)
+				return
+			}
+
+			if !g(fl.To, fs.Newest) {
 				fname := runtime.FuncForPC(reflect.ValueOf(f).Pointer()).Name()
 				log.Lvlf2("verification function failed: %v %s", fname, ver)
 				return false
diff --git a/skipchain/skipchain_test.go b/skipchain/skipchain_test.go
index 8c7c9a9c5a..6fd476b6f2 100644
--- a/skipchain/skipchain_test.go
+++ b/skipchain/skipchain_test.go
@@ -332,6 +332,32 @@ func TestService_ProtocolVerification(t *testing.T) {
 	}
 }
 
+func TestService_ProtocolVerificationPanic(t *testing.T) {
+	// Testing whether we sign correctly the SkipBlocks
+	local := onet.NewLocalTest(cothority.Suite)
+	defer waitPropagationFinished(t, local)
+	defer local.CloseAll()
+	_, el, s := local.MakeSRS(cothority.Suite, 3, skipchainSID)
+	s1 := s.(*Service)
+	verifyFunc := func(newID []byte, newSB *SkipBlock) bool {
+		panic("nope")
+	}
+	verifyID := VerifierID(uuid.NewV1())
+	for _, s := range local.Services {
+		s[skipchainSID].(*Service).registerVerification(verifyID, verifyFunc)
+	}
+
+	sbRoot, err := makeGenesisRosterArgs(s1, el, nil, []VerifierID{verifyID}, 1, 1)
+	require.NoError(t, err)
+	sbNext := sbRoot.Copy()
+	sbNext.BackLinkIDs = []SkipBlockID{sbRoot.Hash}
+	_, err = s1.StoreSkipBlock(&StoreSkipBlock{TargetSkipChainID: sbRoot.Hash, NewBlock: sbNext})
+	require.Error(t, err)
+
+	// We expect a failure here, because the verification function is panicing.
+	require.Contains(t, err.Error(), "couldn't sign forward-link")
+}
+
 func TestService_RegisterVerification(t *testing.T) {
 	// Testing whether we sign correctly the SkipBlocks
 	onet.RegisterNewService("ServiceVerify", newServiceVerify)