feat: harden encoding/decoding functions against panics (#243)

* feat: harden encoding/decoding functions against panics

Part of https://github.com/libp2p/go-libp2p/issues/1389

These kinds of functions:

1. Handle user input.
2. Often have out-of-bounds, null pointer, etc bugs.
3. Have completely isolated logic where local panics are unlikely to
   cause memory corruption elsewhere.

* test: add a panic catcher test

crypto/ecdsa.go

@@ -11,6 +11,7 @@ import (
 	"math/big"
 
 	pb "github.com/libp2p/go-libp2p-core/crypto/pb"
+	"github.com/libp2p/go-libp2p-core/internal/catch"
 
 	"github.com/minio/sha256-simd"
 )
@@ -73,17 +74,21 @@ func ECDSAPublicKeyFromPubKey(pub ecdsa.PublicKey) (PubKey, error) {
 }
 
 // MarshalECDSAPrivateKey returns x509 bytes from a private key
-func MarshalECDSAPrivateKey(ePriv ECDSAPrivateKey) ([]byte, error) {
+func MarshalECDSAPrivateKey(ePriv ECDSAPrivateKey) (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "ECDSA private-key marshal") }()
 	return x509.MarshalECPrivateKey(ePriv.priv)
 }
 
 // MarshalECDSAPublicKey returns x509 bytes from a public key
-func MarshalECDSAPublicKey(ePub ECDSAPublicKey) ([]byte, error) {
+func MarshalECDSAPublicKey(ePub ECDSAPublicKey) (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "ECDSA public-key marshal") }()
 	return x509.MarshalPKIXPublicKey(ePub.pub)
 }
 
 // UnmarshalECDSAPrivateKey returns a private key from x509 bytes
-func UnmarshalECDSAPrivateKey(data []byte) (PrivKey, error) {
+func UnmarshalECDSAPrivateKey(data []byte) (res PrivKey, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "ECDSA private-key unmarshal") }()
+
 	priv, err := x509.ParseECPrivateKey(data)
 	if err != nil {
 		return nil, err
@@ -93,7 +98,9 @@ func UnmarshalECDSAPrivateKey(data []byte) (PrivKey, error) {
 }
 
 // UnmarshalECDSAPublicKey returns the public key from x509 bytes
-func UnmarshalECDSAPublicKey(data []byte) (PubKey, error) {
+func UnmarshalECDSAPublicKey(data []byte) (key PubKey, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "ECDSA public-key unmarshal") }()
+
 	pubIfc, err := x509.ParsePKIXPublicKey(data)
 	if err != nil {
 		return nil, err
@@ -113,7 +120,8 @@ func (ePriv *ECDSAPrivateKey) Type() pb.KeyType {
 }
 
 // Raw returns x509 bytes from a private key
-func (ePriv *ECDSAPrivateKey) Raw() ([]byte, error) {
+func (ePriv *ECDSAPrivateKey) Raw() (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "ECDSA private-key marshal") }()
 	return x509.MarshalECPrivateKey(ePriv.priv)
 }
 
@@ -123,7 +131,8 @@ func (ePriv *ECDSAPrivateKey) Equals(o Key) bool {
 }
 
 // Sign returns the signature of the input data
-func (ePriv *ECDSAPrivateKey) Sign(data []byte) ([]byte, error) {
+func (ePriv *ECDSAPrivateKey) Sign(data []byte) (sig []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "ECDSA signing") }()
 	hash := sha256.Sum256(data)
 	r, s, err := ecdsa.Sign(rand.Reader, ePriv.priv, hash[:])
 	if err != nil {
@@ -157,7 +166,16 @@ func (ePub *ECDSAPublicKey) Equals(o Key) bool {
 }
 
 // Verify compares data to a signature
-func (ePub *ECDSAPublicKey) Verify(data, sigBytes []byte) (bool, error) {
+func (ePub *ECDSAPublicKey) Verify(data, sigBytes []byte) (success bool, err error) {
+	defer func() {
+		catch.HandlePanic(recover(), &err, "ECDSA signature verification")
+
+		// Just to be extra paranoid.
+		if err != nil {
+			success = false
+		}
+	}()
+
 	sig := new(ECDSASig)
 	if _, err := asn1.Unmarshal(sigBytes, sig); err != nil {
 		return false, err

crypto/ed25519.go

@@ -9,6 +9,7 @@ import (
 	"io"
 
 	pb "github.com/libp2p/go-libp2p-core/crypto/pb"
+	"github.com/libp2p/go-libp2p-core/internal/catch"
 )
 
 // Ed25519PrivateKey is an ed25519 private key.
@@ -74,7 +75,9 @@ func (k *Ed25519PrivateKey) GetPublic() PubKey {
 }
 
 // Sign returns a signature from an input message.
-func (k *Ed25519PrivateKey) Sign(msg []byte) ([]byte, error) {
+func (k *Ed25519PrivateKey) Sign(msg []byte) (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "ed15519 signing") }()
+
 	return ed25519.Sign(k.k, msg), nil
 }
 
@@ -99,7 +102,15 @@ func (k *Ed25519PublicKey) Equals(o Key) bool {
 }
 
 // Verify checks a signature agains the input data.
-func (k *Ed25519PublicKey) Verify(data []byte, sig []byte) (bool, error) {
+func (k *Ed25519PublicKey) Verify(data []byte, sig []byte) (success bool, err error) {
+	defer func() {
+		catch.HandlePanic(recover(), &err, "ed15519 signature verification")
+
+		// To be safe.
+		if err != nil {
+			success = false
+		}
+	}()
 	return ed25519.Verify(k.k, data, sig), nil
 }
 

crypto/key_openssl.go

@@ -12,16 +12,19 @@ import (
 
 	btcec "github.com/btcsuite/btcd/btcec"
 	openssl "github.com/libp2p/go-openssl"
+
+	"github.com/libp2p/go-libp2p-core/internal/catch"
 )
 
 // KeyPairFromStdKey wraps standard library (and secp256k1) private keys in libp2p/go-libp2p-core/crypto keys
-func KeyPairFromStdKey(priv crypto.PrivateKey) (PrivKey, PubKey, error) {
+func KeyPairFromStdKey(priv crypto.PrivateKey) (_priv PrivKey, _pub PubKey, err error) {
 	if priv == nil {
 		return nil, nil, ErrNilPrivateKey
 	}
 
 	switch p := priv.(type) {
 	case *rsa.PrivateKey:
+		defer func() { catch.HandlePanic(recover(), &err, "x509 private key marshaling") }()
 		pk, err := openssl.LoadPrivateKeyFromDER(x509.MarshalPKCS1PrivateKey(p))
 		if err != nil {
 			return nil, nil, err
@@ -48,12 +51,13 @@ func KeyPairFromStdKey(priv crypto.PrivateKey) (PrivKey, PubKey, error) {
 }
 
 // PrivKeyToStdKey converts libp2p/go-libp2p-core/crypto private keys to standard library (and secp256k1) private keys
-func PrivKeyToStdKey(priv PrivKey) (crypto.PrivateKey, error) {
+func PrivKeyToStdKey(priv PrivKey) (_priv crypto.PrivateKey, err error) {
 	if priv == nil {
 		return nil, ErrNilPrivateKey
 	}
 	switch p := priv.(type) {
 	case *opensslPrivateKey:
+		defer func() { catch.HandlePanic(recover(), &err, "x509 private key parsing") }()
 		raw, err := p.Raw()
 		if err != nil {
 			return nil, err
@@ -71,13 +75,15 @@ func PrivKeyToStdKey(priv PrivKey) (crypto.PrivateKey, error) {
 }
 
 // PubKeyToStdKey converts libp2p/go-libp2p-core/crypto private keys to standard library (and secp256k1) public keys
-func PubKeyToStdKey(pub PubKey) (crypto.PublicKey, error) {
+func PubKeyToStdKey(pub PubKey) (key crypto.PublicKey, err error) {
 	if pub == nil {
 		return nil, ErrNilPublicKey
 	}
 
 	switch p := pub.(type) {
 	case *opensslPublicKey:
+		defer func() { catch.HandlePanic(recover(), &err, "x509 public key parsing") }()
+
 		raw, err := p.Raw()
 		if err != nil {
 			return nil, err

crypto/rsa_go.go

@@ -12,6 +12,7 @@ import (
 	"io"
 
 	pb "github.com/libp2p/go-libp2p-core/crypto/pb"
+	"github.com/libp2p/go-libp2p-core/internal/catch"
 
 	"github.com/minio/sha256-simd"
 )
@@ -42,9 +43,17 @@ func GenerateRSAKeyPair(bits int, src io.Reader) (PrivKey, PubKey, error) {
 }
 
 // Verify compares a signature against input data
-func (pk *RsaPublicKey) Verify(data, sig []byte) (bool, error) {
+func (pk *RsaPublicKey) Verify(data, sig []byte) (success bool, err error) {
+	defer func() {
+		catch.HandlePanic(recover(), &err, "RSA signature verification")
+
+		// To be safe
+		if err != nil {
+			success = false
+		}
+	}()
 	hashed := sha256.Sum256(data)
-	err := rsa.VerifyPKCS1v15(&pk.k, crypto.SHA256, hashed[:], sig)
+	err = rsa.VerifyPKCS1v15(&pk.k, crypto.SHA256, hashed[:], sig)
 	if err != nil {
 		return false, err
 	}
@@ -55,7 +64,8 @@ func (pk *RsaPublicKey) Type() pb.KeyType {
 	return pb.KeyType_RSA
 }
 
-func (pk *RsaPublicKey) Raw() ([]byte, error) {
+func (pk *RsaPublicKey) Raw() (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "RSA public-key marshaling") }()
 	return x509.MarshalPKIXPublicKey(&pk.k)
 }
 
@@ -71,7 +81,8 @@ func (pk *RsaPublicKey) Equals(k Key) bool {
 }
 
 // Sign returns a signature of the input data
-func (sk *RsaPrivateKey) Sign(message []byte) ([]byte, error) {
+func (sk *RsaPrivateKey) Sign(message []byte) (sig []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "RSA signing") }()
 	hashed := sha256.Sum256(message)
 	return rsa.SignPKCS1v15(rand.Reader, &sk.sk, crypto.SHA256, hashed[:])
 }
@@ -85,7 +96,8 @@ func (sk *RsaPrivateKey) Type() pb.KeyType {
 	return pb.KeyType_RSA
 }
 
-func (sk *RsaPrivateKey) Raw() ([]byte, error) {
+func (sk *RsaPrivateKey) Raw() (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "RSA private-key marshaling") }()
 	b := x509.MarshalPKCS1PrivateKey(&sk.sk)
 	return b, nil
 }
@@ -106,7 +118,8 @@ func (sk *RsaPrivateKey) Equals(k Key) bool {
 }
 
 // UnmarshalRsaPrivateKey returns a private key from the input x509 bytes
-func UnmarshalRsaPrivateKey(b []byte) (PrivKey, error) {
+func UnmarshalRsaPrivateKey(b []byte) (key PrivKey, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "RSA private-key unmarshaling") }()
 	sk, err := x509.ParsePKCS1PrivateKey(b)
 	if err != nil {
 		return nil, err
@@ -118,7 +131,8 @@ func UnmarshalRsaPrivateKey(b []byte) (PrivKey, error) {
 }
 
 // UnmarshalRsaPublicKey returns a public key from the input x509 bytes
-func UnmarshalRsaPublicKey(b []byte) (PubKey, error) {
+func UnmarshalRsaPublicKey(b []byte) (key PubKey, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "RSA public-key unmarshaling") }()
 	pub, err := x509.ParsePKIXPublicKey(b)
 	if err != nil {
 		return nil, err

crypto/secp256k1.go

@@ -5,6 +5,7 @@ import (
 	"io"
 
 	pb "github.com/libp2p/go-libp2p-core/crypto/pb"
+	"github.com/libp2p/go-libp2p-core/internal/catch"
 
 	"github.com/btcsuite/btcd/btcec"
 	"github.com/minio/sha256-simd"
@@ -28,17 +29,19 @@ func GenerateSecp256k1Key(src io.Reader) (PrivKey, PubKey, error) {
 }
 
 // UnmarshalSecp256k1PrivateKey returns a private key from bytes
-func UnmarshalSecp256k1PrivateKey(data []byte) (PrivKey, error) {
+func UnmarshalSecp256k1PrivateKey(data []byte) (k PrivKey, err error) {
 	if len(data) != btcec.PrivKeyBytesLen {
 		return nil, fmt.Errorf("expected secp256k1 data size to be %d", btcec.PrivKeyBytesLen)
 	}
+	defer func() { catch.HandlePanic(recover(), &err, "secp256k1 private-key unmarshal") }()
 
 	privk, _ := btcec.PrivKeyFromBytes(btcec.S256(), data)
 	return (*Secp256k1PrivateKey)(privk), nil
 }
 
 // UnmarshalSecp256k1PublicKey returns a public key from bytes
-func UnmarshalSecp256k1PublicKey(data []byte) (PubKey, error) {
+func UnmarshalSecp256k1PublicKey(data []byte) (_k PubKey, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "secp256k1 public-key unmarshal") }()
 	k, err := btcec.ParsePubKey(data, btcec.S256())
 	if err != nil {
 		return nil, err
@@ -68,7 +71,8 @@ func (k *Secp256k1PrivateKey) Equals(o Key) bool {
 }
 
 // Sign returns a signature from input data
-func (k *Secp256k1PrivateKey) Sign(data []byte) ([]byte, error) {
+func (k *Secp256k1PrivateKey) Sign(data []byte) (_sig []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "secp256k1 signing") }()
 	hash := sha256.Sum256(data)
 	sig, err := (*btcec.PrivateKey)(k).Sign(hash[:])
 	if err != nil {
@@ -89,7 +93,8 @@ func (k *Secp256k1PublicKey) Type() pb.KeyType {
 }
 
 // Raw returns the bytes of the key
-func (k *Secp256k1PublicKey) Raw() ([]byte, error) {
+func (k *Secp256k1PublicKey) Raw() (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "secp256k1 public key marshaling") }()
 	return (*btcec.PublicKey)(k).SerializeCompressed(), nil
 }
 
@@ -104,7 +109,15 @@ func (k *Secp256k1PublicKey) Equals(o Key) bool {
 }
 
 // Verify compares a signature against the input data
-func (k *Secp256k1PublicKey) Verify(data []byte, sigStr []byte) (bool, error) {
+func (k *Secp256k1PublicKey) Verify(data []byte, sigStr []byte) (success bool, err error) {
+	defer func() {
+		catch.HandlePanic(recover(), &err, "secp256k1 signature verification")
+
+		// To be extra safe.
+		if err != nil {
+			success = false
+		}
+	}()
 	sig, err := btcec.ParseDERSignature(sigStr, btcec.S256())
 	if err != nil {
 		return false, err

internal/catch/catch.go

@@ -0,0 +1,18 @@
+package catch
+
+import (
+	"fmt"
+	"io"
+	"os"
+	"runtime/debug"
+)
+
+var panicWriter io.Writer = os.Stderr
+
+// HandlePanic handles and logs panics.
+func HandlePanic(rerr interface{}, err *error, where string) {
+	if rerr != nil {
+		fmt.Fprintf(panicWriter, "caught panic: %s\n%s\n", rerr, debug.Stack())
+		*err = fmt.Errorf("panic in %s: %s", where, rerr)
+	}
+}

internal/catch/catch_test.go

@@ -0,0 +1,28 @@
+package catch
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCatch(t *testing.T) {
+	buf := new(bytes.Buffer)
+
+	oldPanicWriter := panicWriter
+	t.Cleanup(func() { panicWriter = oldPanicWriter })
+	panicWriter = buf
+
+	panicAndCatch := func() (err error) {
+		defer func() { HandlePanic(recover(), &err, "somewhere") }()
+
+		panic("here")
+	}
+
+	err := panicAndCatch()
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "panic in somewhere: here")
+
+	require.Contains(t, buf.String(), "caught panic: here")
+}

peer/addrinfo_serde.go

@@ -3,6 +3,7 @@ package peer
 import (
 	"encoding/json"
 
+	"github.com/libp2p/go-libp2p-core/internal/catch"
 	ma "github.com/multiformats/go-multiaddr"
 )
 
@@ -12,7 +13,9 @@ type addrInfoJson struct {
 	Addrs []string
 }
 
-func (pi AddrInfo) MarshalJSON() ([]byte, error) {
+func (pi AddrInfo) MarshalJSON() (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "libp2p addr info marshal") }()
+
 	addrs := make([]string, len(pi.Addrs))
 	for i, addr := range pi.Addrs {
 		addrs[i] = addr.String()
@@ -23,7 +26,8 @@ func (pi AddrInfo) MarshalJSON() ([]byte, error) {
 	})
 }
 
-func (pi *AddrInfo) UnmarshalJSON(b []byte) error {
+func (pi *AddrInfo) UnmarshalJSON(b []byte) (err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "libp2p addr info unmarshal") }()
 	var data addrInfoJson
 	if err := json.Unmarshal(b, &data); err != nil {
 		return err

peer/record.go

@@ -5,6 +5,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/libp2p/go-libp2p-core/internal/catch"
 	pb "github.com/libp2p/go-libp2p-core/peer/pb"
 	"github.com/libp2p/go-libp2p-core/record"
 
@@ -160,13 +161,15 @@ func (r *PeerRecord) Codec() []byte {
 // This method is called automatically when consuming a record.Envelope
 // whose PayloadType indicates that it contains a PeerRecord.
 // It is generally not necessary or recommended to call this method directly.
-func (r *PeerRecord) UnmarshalRecord(bytes []byte) error {
+func (r *PeerRecord) UnmarshalRecord(bytes []byte) (err error) {
 	if r == nil {
 		return fmt.Errorf("cannot unmarshal PeerRecord to nil receiver")
 	}
 
+	defer func() { catch.HandlePanic(recover(), &err, "libp2p peer record unmarshal") }()
+
 	var msg pb.PeerRecord
-	err := proto.Unmarshal(bytes, &msg)
+	err = proto.Unmarshal(bytes, &msg)
 	if err != nil {
 		return err
 	}
@@ -183,7 +186,9 @@ func (r *PeerRecord) UnmarshalRecord(bytes []byte) error {
 // MarshalRecord serializes a PeerRecord to a byte slice.
 // This method is called automatically when constructing a routing.Envelope
 // using Seal or PeerRecord.Sign.
-func (r *PeerRecord) MarshalRecord() ([]byte, error) {
+func (r *PeerRecord) MarshalRecord() (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "libp2p peer record marshal") }()
+
 	msg, err := r.ToProtobuf()
 	if err != nil {
 		return nil, err

record/envelope.go

@@ -7,6 +7,7 @@ import (
 	"sync"
 
 	"github.com/libp2p/go-libp2p-core/crypto"
+	"github.com/libp2p/go-libp2p-core/internal/catch"
 	pb "github.com/libp2p/go-libp2p-core/record/pb"
 
 	pool "github.com/libp2p/go-buffer-pool"
@@ -192,7 +193,8 @@ func UnmarshalEnvelope(data []byte) (*Envelope, error) {
 
 // Marshal returns a byte slice containing a serialized protobuf representation
 // of a Envelope.
-func (e *Envelope) Marshal() ([]byte, error) {
+func (e *Envelope) Marshal() (res []byte, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "libp2p envelope marshal") }()
 	key, err := crypto.PublicKeyToProto(e.PublicKey)
 	if err != nil {
 		return nil, err

record/record.go

@@ -3,6 +3,8 @@ package record
 import (
 	"errors"
 	"reflect"
+
+	"github.com/libp2p/go-libp2p-core/internal/catch"
 )
 
 var (
@@ -70,7 +72,9 @@ func RegisterType(prototype Record) {
 	payloadTypeRegistry[string(prototype.Codec())] = getValueType(prototype)
 }
 
-func unmarshalRecordPayload(payloadType []byte, payloadBytes []byte) (Record, error) {
+func unmarshalRecordPayload(payloadType []byte, payloadBytes []byte) (_rec Record, err error) {
+	defer func() { catch.HandlePanic(recover(), &err, "libp2p envelope record unmarshal") }()
+
 	rec, err := blankRecordForPayloadType(payloadType)
 	if err != nil {
 		return nil, err