feat: harden encoding/decoding functions against panics

Part of libp2p/go-libp2p#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.
libp2p · Apr 16, 2022 · bee2c55 · bee2c55
1 parent 2a28e1d
commit bee2c55
Show file tree

Hide file tree

Showing 10 changed files with 122 additions and 31 deletions.
diff --git a/crypto/ecdsa.go b/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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&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.CatchAndLog(&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

diff --git a/crypto/ed25519.go b/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 catch.CatchAndLog(&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.CatchAndLog(&err, "ed15519 signature verification")
+
+		// To be safe.
+		if err != nil {
+			success = false
+		}
+	}()
 	return ed25519.Verify(k.k, data, sig), nil
 }
 

diff --git a/crypto/key_openssl.go b/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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&err, "x509 public key parsing")
+
 		raw, err := p.Raw()
 		if err != nil {
 			return nil, err

diff --git a/crypto/rsa_go.go b/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.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&err, "RSA public-key unmarshaling")
 	pub, err := x509.ParsePKIXPublicKey(b)
 	if err != nil {
 		return nil, err

diff --git a/crypto/secp256k1.go b/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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&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 catch.CatchAndLog(&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.CatchAndLog(&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

diff --git a/internal/catch/catch.go b/internal/catch/catch.go
@@ -0,0 +1,14 @@
+package catch
+
+import (
+	"fmt"
+	"os"
+	"runtime/debug"
+)
+
+func CatchAndLog(err *error, where string) {
+	if rerr := recover(); rerr != nil {
+		fmt.Fprintf(os.Stderr, "caught panic: %s\n%s\n", rerr, debug.Stack())
+		*err = fmt.Errorf("panic in %s: %s", where, rerr)
+	}
+}
diff --git a/peer/addrinfo_serde.go b/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 catch.CatchAndLog(&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 catch.CatchAndLog(&err, "libp2p addr info unmarshal")
 	var data addrInfoJson
 	if err := json.Unmarshal(b, &data); err != nil {
 		return err

diff --git a/peer/record.go b/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 catch.CatchAndLog(&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 catch.CatchAndLog(&err, "libp2p peer record marshal")
+
 	msg, err := r.ToProtobuf()
 	if err != nil {
 		return nil, err