Ethereum address creation faulty (#345)

* Fix faulty hashing of ethereum address

pkg/bzz/address.go

@@ -59,7 +59,10 @@ func ParseAddress(underlay, overlay, signature []byte, networkID uint64) (*Addre
 		return nil, ErrInvalidAddress
 	}
 
-	recoveredOverlay := crypto.NewOverlayAddress(*recoveredPK, networkID)
+	recoveredOverlay, err := crypto.NewOverlayAddress(*recoveredPK, networkID)
+	if err != nil {
+		return nil, ErrInvalidAddress
+	}
 	if !bytes.Equal(recoveredOverlay.Bytes(), overlay) {
 		return nil, ErrInvalidAddress
 	}

pkg/bzz/address_test.go

@@ -24,7 +24,10 @@ func TestBzzAddress(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	overlay := crypto.NewOverlayAddress(privateKey1.PublicKey, 3)
+	overlay, err := crypto.NewOverlayAddress(privateKey1.PublicKey, 3)
+	if err != nil {
+		t.Fatal(err)
+	}
 	signer1 := crypto.NewDefaultSigner(privateKey1)
 
 	bzzAddress, err := bzz.NewAddress(signer1, node1ma, overlay, 3)

pkg/crypto/crypto.go

@@ -9,6 +9,7 @@ import (
 	"crypto/elliptic"
 	"crypto/rand"
 	"encoding/binary"
+	"errors"
 	"fmt"
 
 	"github.com/btcsuite/btcd/btcec"
@@ -17,12 +18,15 @@ import (
 )
 
 // NewOverlayAddress constructs a Swarm Address from ECDSA private key.
-func NewOverlayAddress(p ecdsa.PublicKey, networkID uint64) swarm.Address {
-	ethAddr := NewEthereumAddress(p)
+func NewOverlayAddress(p ecdsa.PublicKey, networkID uint64) (swarm.Address, error) {
+	ethAddr, err := NewEthereumAddress(p)
+	if err != nil {
+		return swarm.ZeroAddress, err
+	}
 	netIDBytes := make([]byte, 8)
 	binary.LittleEndian.PutUint64(netIDBytes, networkID)
 	h := sha3.Sum256(append(ethAddr, netIDBytes...))
-	return swarm.NewAddress(h[:])
+	return swarm.NewAddress(h[:]), nil
 }
 
 // GenerateSecp256k1Key generates an ECDSA private key using
@@ -47,14 +51,24 @@ func DecodeSecp256k1PrivateKey(data []byte) (*ecdsa.PrivateKey, error) {
 
 // NewEthereumAddress returns a binary representation of ethereum blockchain address.
 // This function is based on github.com/ethereum/go-ethereum/crypto.PubkeyToAddress.
-func NewEthereumAddress(p ecdsa.PublicKey) []byte {
+func NewEthereumAddress(p ecdsa.PublicKey) ([]byte, error) {
 	if p.X == nil || p.Y == nil {
-		return nil
+		return nil, errors.New("invalid public key")
 	}
 	pubBytes := elliptic.Marshal(btcec.S256(), p.X, p.Y)
-	return legacyKeccak256(pubBytes[1:])[12:]
+	pubHash, err := legacyKeccak256(pubBytes[1:])
+	if err != nil {
+		return nil, err
+	}
+	return pubHash[12:], err
 }
 
-func legacyKeccak256(data []byte) []byte {
-	return sha3.NewLegacyKeccak256().Sum(data)
+func legacyKeccak256(data []byte) ([]byte, error) {
+	var err error
+	hasher := sha3.NewLegacyKeccak256()
+	_, err = hasher.Write(data)
+	if err != nil {
+		return nil, err
+	}
+	return hasher.Sum(nil), err
 }

pkg/crypto/crypto_test.go

@@ -6,6 +6,7 @@ package crypto_test
 
 import (
 	"bytes"
+	"encoding/hex"
 	"testing"
 
 	"github.com/ethersphere/bee/pkg/crypto"
@@ -37,7 +38,10 @@ func TestNewAddress(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	a := crypto.NewOverlayAddress(k.PublicKey, 1)
+	a, err := crypto.NewOverlayAddress(k.PublicKey, 1)
+	if err != nil {
+		t.Fatal(err)
+	}
 	if l := len(a.Bytes()); l != 32 {
 		t.Errorf("got address length %v, want %v", l, 32)
 	}
@@ -57,3 +61,27 @@ func TestEncodeSecp256k1PrivateKey(t *testing.T) {
 		t.Fatal("encoded and decoded keys are not equal")
 	}
 }
+
+func TestNewEthereumAddress(t *testing.T) {
+	privKeyHex := "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae"
+	privKeyBytes, err := hex.DecodeString(privKeyHex)
+	if err != nil {
+		t.Fatal(err)
+	}
+	privKey, err := crypto.DecodeSecp256k1PrivateKey(privKeyBytes)
+	if err != nil {
+		t.Fatal(err)
+	}
+	expectAddressHex := "2f63cbeb054ce76050827e42dd75268f6b9d87c5"
+	expectAddress, err := hex.DecodeString(expectAddressHex)
+	if err != nil {
+		t.Fatal(err)
+	}
+	address, err := crypto.NewEthereumAddress(privKey.PublicKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(address, expectAddress) {
+		t.Fatalf("address mismatch %x %x", address, expectAddress)
+	}
+}

pkg/debugapi/peer_test.go

@@ -32,7 +32,10 @@ func TestConnect(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	overlay := crypto.NewOverlayAddress(privateKey.PublicKey, 0)
+	overlay, err := crypto.NewOverlayAddress(privateKey.PublicKey, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
 	underlama, err := ma.NewMultiaddr(underlay)
 	if err != nil {
 		t.Fatal(err)

pkg/hive/hive_test.go

@@ -57,7 +57,10 @@ func TestBroadcastPeers(t *testing.T) {
 			t.Fatal(err)
 		}
 		signer := crypto.NewDefaultSigner(pk)
-		overlay := crypto.NewOverlayAddress(pk.PublicKey, networkID)
+		overlay, err := crypto.NewOverlayAddress(pk.PublicKey, networkID)
+		if err != nil {
+			t.Fatal(err)
+		}
 		bzzAddr, err := bzz.NewAddress(signer, underlay, overlay, networkID)
 		if err != nil {
 			t.Fatal(err)

pkg/keystore/file/key.go

@@ -67,8 +67,12 @@ func encryptKey(k *ecdsa.PrivateKey, password string) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
+	addr, err := crypto.NewEthereumAddress(k.PublicKey)
+	if err != nil {
+		return nil, err
+	}
 	return json.Marshal(encryptedKey{
-		Address: hex.EncodeToString(crypto.NewEthereumAddress(k.PublicKey)),
+		Address: hex.EncodeToString(addr),
 		Crypto:  *kc,
 		Version: keyVersion,
 	})

pkg/node/node.go

@@ -116,7 +116,10 @@ func NewBee(o Options) (*Bee, error) {
 	if err != nil {
 		return nil, fmt.Errorf("swarm key: %w", err)
 	}
-	address := crypto.NewOverlayAddress(swarmPrivateKey.PublicKey, o.NetworkID)
+	address, err := crypto.NewOverlayAddress(swarmPrivateKey.PublicKey, o.NetworkID)
+	if err != nil {
+		return nil, err
+	}
 	if created {
 		logger.Infof("new swarm network address created: %s", address)
 	} else {

pkg/p2p/libp2p/internal/handshake/handshake_test.go

@@ -59,11 +59,19 @@ func TestHandshake(t *testing.T) {
 
 	signer1 := crypto.NewDefaultSigner(privateKey1)
 	signer2 := crypto.NewDefaultSigner(privateKey2)
-	node1BzzAddress, err := bzz.NewAddress(signer1, node1ma, crypto.NewOverlayAddress(privateKey1.PublicKey, networkID), networkID)
+	addr, err := crypto.NewOverlayAddress(privateKey1.PublicKey, networkID)
 	if err != nil {
 		t.Fatal(err)
 	}
-	node2BzzAddress, err := bzz.NewAddress(signer2, node2ma, crypto.NewOverlayAddress(privateKey2.PublicKey, networkID), networkID)
+	node1BzzAddress, err := bzz.NewAddress(signer1, node1ma, addr, networkID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	addr2, err := crypto.NewOverlayAddress(privateKey2.PublicKey, networkID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	node2BzzAddress, err := bzz.NewAddress(signer2, node2ma, addr2, networkID)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -580,7 +588,7 @@ func TestHandshake(t *testing.T) {
 
 		_, err = handshakeService.Handle(stream1, node2AddrInfo.Addrs[0], node2AddrInfo.ID)
 		if err != handshake.ErrInvalidAck {
-			t.Fatalf("expected %s, got %s", handshake.ErrInvalidAck, err)
+			t.Fatalf("expected %s, got %v", handshake.ErrInvalidAck, err)
 		}
 	})
 

pkg/p2p/libp2p/libp2p_test.go

@@ -31,7 +31,10 @@ func newService(t *testing.T, networkID uint64, o libp2p.Options) (s *libp2p.Ser
 		t.Fatal(err)
 	}
 
-	overlay = crypto.NewOverlayAddress(swarmKey.PublicKey, networkID)
+	overlay, err = crypto.NewOverlayAddress(swarmKey.PublicKey, networkID)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	addr := ":0"