Merge pull request #4930 from onflow/petera/4928-rest-address-validation

[Access] Validate addresses match network in rest api
onflow · Nov 7, 2023 · d13d4be · d13d4be
2 parents 6591e54 + d2d53b6
commit d13d4be
Show file tree

Hide file tree

Showing 11 changed files with 87 additions and 32 deletions.
diff --git a/engine/access/rest/request/address.go b/engine/access/rest/request/address.go
@@ -5,16 +5,22 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/onflow/flow-go/engine/common/rpc/convert"
 	"github.com/onflow/flow-go/model/flow"
 )
 
-func ParseAddress(raw string) (flow.Address, error) {
+func ParseAddress(raw string, chain flow.Chain) (flow.Address, error) {
 	raw = strings.ReplaceAll(raw, "0x", "") // remove 0x prefix
 
 	valid, _ := regexp.MatchString(`^[0-9a-fA-F]{16}$`, raw)
 	if !valid {
 		return flow.EmptyAddress, fmt.Errorf("invalid address")
 	}
 
-	return flow.HexToAddress(raw), nil
+	address, err := convert.HexToAddress(raw, chain)
+	if err != nil {
+		return flow.EmptyAddress, err
+	}
+
+	return address, nil
 }
diff --git a/engine/access/rest/request/address_test.go b/engine/access/rest/request/address_test.go
@@ -5,6 +5,11 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+
+	"github.com/onflow/flow-go/model/flow"
 )
 
 func TestAddress_InvalidParse(t *testing.T) {
@@ -14,25 +19,41 @@ func TestAddress_InvalidParse(t *testing.T) {
 		"foo",
 		"1",
 		"@",
-		"ead892083b3e2c61222",
+		"ead892083b3e2c61222", // too long
 	}
 
+	chain := flow.Localnet.Chain()
 	for _, input := range inputs {
-		_, err := ParseAddress(input)
+		_, err := ParseAddress(input, chain)
 		assert.EqualError(t, err, "invalid address")
 	}
 }
 
+func TestAddress_InvalidNetwork(t *testing.T) {
+	inputs := []string{
+		"18eb4ee6b3c026d2",
+		"0x18eb4ee6b3c026d2",
+	}
+
+	chain := flow.Localnet.Chain()
+	for _, input := range inputs {
+		_, err := ParseAddress(input, chain)
+		require.Error(t, err)
+		assert.Equal(t, codes.InvalidArgument, status.Code(err))
+	}
+}
+
 func TestAddress_ValidParse(t *testing.T) {
 	inputs := []string{
 		"f8d6e0586b0a20c7",
-		"f3ad66eea58c97d2",
-		"0xead892083b3e2c6c",
+		"148602c0600814da",
+		"0x0b807ae5da6210df",
 	}
 
+	chain := flow.Localnet.Chain()
 	for _, input := range inputs {
-		address, err := ParseAddress(input)
-		assert.NoError(t, err)
+		address, err := ParseAddress(input, chain)
+		require.NoError(t, err)
 		assert.Equal(t, strings.ReplaceAll(input, "0x", ""), address.String())
 	}
 }
diff --git a/engine/access/rest/request/get_account.go b/engine/access/rest/request/get_account.go
@@ -16,11 +16,12 @@ func (g *GetAccount) Build(r *Request) error {
 	return g.Parse(
 		r.GetVar(addressVar),
 		r.GetQueryParam(blockHeightQuery),
+		r.Chain,
 	)
 }
 
-func (g *GetAccount) Parse(rawAddress string, rawHeight string) error {
-	address, err := ParseAddress(rawAddress)
+func (g *GetAccount) Parse(rawAddress string, rawHeight string, chain flow.Chain) error {
+	address, err := ParseAddress(rawAddress, chain)
 	if err != nil {
 		return err
 	}

diff --git a/engine/access/rest/request/get_account_key.go b/engine/access/rest/request/get_account_key.go
@@ -20,15 +20,17 @@ func (g *GetAccountKey) Build(r *Request) error {
 		r.GetVar(addressVar),
 		r.GetVar(indexVar),
 		r.GetQueryParam(blockHeightQuery),
+		r.Chain,
 	)
 }
 
 func (g *GetAccountKey) Parse(
 	rawAddress string,
 	rawIndex string,
 	rawHeight string,
+	chain flow.Chain,
 ) error {
-	address, err := ParseAddress(rawAddress)
+	address, err := ParseAddress(rawAddress, chain)
 	if err != nil {
 		return err
 	}

diff --git a/engine/access/rest/request/get_account_key_test.go b/engine/access/rest/request/get_account_key_test.go
@@ -4,6 +4,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"github.com/onflow/flow-go/model/flow"
 )
 
 func Test_GetAccountKey_InvalidParse(t *testing.T) {
@@ -39,9 +41,10 @@ func Test_GetAccountKey_InvalidParse(t *testing.T) {
 		},
 	}
 
+	chain := flow.Localnet.Chain()
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err := getAccountKey.Parse(test.address, test.index, test.height)
+			err := getAccountKey.Parse(test.address, test.index, test.height, chain)
 			assert.EqualError(t, err, test.err)
 		})
 	}
@@ -53,25 +56,26 @@ func Test_GetAccountKey_ValidParse(t *testing.T) {
 	addr := "f8d6e0586b0a20c7"
 	keyIndex := "5"
 	height := "100"
-	err := getAccountKey.Parse(addr, keyIndex, height)
+	chain := flow.Localnet.Chain()
+	err := getAccountKey.Parse(addr, keyIndex, height, chain)
 	assert.NoError(t, err)
 	assert.Equal(t, getAccountKey.Address.String(), addr)
 	assert.Equal(t, getAccountKey.Index, uint64(5))
 	assert.Equal(t, getAccountKey.Height, uint64(100))
 
-	err = getAccountKey.Parse(addr, keyIndex, "")
+	err = getAccountKey.Parse(addr, keyIndex, "", chain)
 	assert.NoError(t, err)
 	assert.Equal(t, getAccountKey.Address.String(), addr)
 	assert.Equal(t, getAccountKey.Index, uint64(5))
 	assert.Equal(t, getAccountKey.Height, SealedHeight)
 
-	err = getAccountKey.Parse(addr, keyIndex, "sealed")
+	err = getAccountKey.Parse(addr, keyIndex, "sealed", chain)
 	assert.NoError(t, err)
 	assert.Equal(t, getAccountKey.Address.String(), addr)
 	assert.Equal(t, getAccountKey.Index, uint64(5))
 	assert.Equal(t, getAccountKey.Height, SealedHeight)
 
-	err = getAccountKey.Parse(addr, keyIndex, "final")
+	err = getAccountKey.Parse(addr, keyIndex, "final", chain)
 	assert.NoError(t, err)
 	assert.Equal(t, getAccountKey.Address.String(), addr)
 	assert.Equal(t, getAccountKey.Index, uint64(5))

diff --git a/engine/access/rest/request/get_account_test.go b/engine/access/rest/request/get_account_test.go
@@ -5,6 +5,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"github.com/onflow/flow-go/model/flow"
 )
 
 func Test_GetAccount_InvalidParse(t *testing.T) {
@@ -19,8 +21,9 @@ func Test_GetAccount_InvalidParse(t *testing.T) {
 		{"f8d6e0586b0a20c7", "-1", "invalid height format"},
 	}
 
+	chain := flow.Localnet.Chain()
 	for i, test := range tests {
-		err := getAccount.Parse(test.address, test.height)
+		err := getAccount.Parse(test.address, test.height, chain)
 		assert.EqualError(t, err, test.err, fmt.Sprintf("test #%d failed", i))
 	}
 }
@@ -29,12 +32,13 @@ func Test_GetAccount_ValidParse(t *testing.T) {
 	var getAccount GetAccount
 
 	addr := "f8d6e0586b0a20c7"
-	err := getAccount.Parse(addr, "")
+	chain := flow.Localnet.Chain()
+	err := getAccount.Parse(addr, "", chain)
 	assert.NoError(t, err)
 	assert.Equal(t, getAccount.Address.String(), addr)
 	assert.Equal(t, getAccount.Height, SealedHeight)
 
-	err = getAccount.Parse(addr, "100")
+	err = getAccount.Parse(addr, "100", chain)
 	assert.NoError(t, err)
 	assert.Equal(t, getAccount.Height, uint64(100))
 }
diff --git a/engine/access/rest/request/proposal_key.go b/engine/access/rest/request/proposal_key.go
@@ -10,8 +10,8 @@ import (
 
 type ProposalKey flow.ProposalKey
 
-func (p *ProposalKey) Parse(raw models.ProposalKey) error {
-	address, err := ParseAddress(raw.Address)
+func (p *ProposalKey) Parse(raw models.ProposalKey, chain flow.Chain) error {
+	address, err := ParseAddress(raw.Address, chain)
 	if err != nil {
 		return err
 	}

diff --git a/engine/access/rest/request/signature_test.go b/engine/access/rest/request/signature_test.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/onflow/flow-go/engine/access/rest/models"
 	"github.com/onflow/flow-go/engine/access/rest/util"
+	"github.com/onflow/flow-go/model/flow"
 )
 
 func TestSignature_InvalidParse(t *testing.T) {
@@ -42,7 +43,7 @@ func TestTransactionSignature_ValidParse(t *testing.T) {
 	sig := "c83665f5212fad065cd27d370ef80e5fbdd20cd57411af5c76076a15dced05ac6e6d9afa88cd7337bf9c869f6785ecc1c568ca593a99dfeec14e024c0cd78289"
 	sigHex, _ := hex.DecodeString(sig)
 	encodedSig := util.ToBase64(sigHex)
-	err := txSignature.Parse(addr, "0", encodedSig)
+	err := txSignature.Parse(addr, "0", encodedSig, flow.Localnet.Chain())
 
 	assert.NoError(t, err)
 	assert.Equal(t, addr, txSignature.Address.String())
@@ -57,10 +58,11 @@ func TestTransactionSignatures_ValidParse(t *testing.T) {
 		inSigs      []string
 	}{
 		{[]string{"01cf0e2f2f715450"}, []string{"c83665f5212fad065cd27d370ef80e5fbdd20cd57411af5c76076a15dced05ac6e6d9afa88cd7337bf9c869f6785ecc1c568ca593a99dfeec14e024c0cd78289"}},
-		{[]string{"51cf0e2f2f715450", "21cf0e2f2f715454"}, []string{"223665f5212fad065cd27d370ef80e5fbdd20cd57411af5c76076a15dced05ac6e6d9afa88cd7337bf9c869f6785ecc1c568ca593a99dfeec14e024c0cd78289", "5553665f5212fad065cd27d370ef80e5fbdd20cd57411af5c76076a15dced05ac6e6d9afa88cd7337bf9c869f6785ecc1c568ca593a99dfeec14e024c0cd7822"}},
+		{[]string{"ee82856bf20e2aa6", "e03daebed8ca0615"}, []string{"223665f5212fad065cd27d370ef80e5fbdd20cd57411af5c76076a15dced05ac6e6d9afa88cd7337bf9c869f6785ecc1c568ca593a99dfeec14e024c0cd78289", "5553665f5212fad065cd27d370ef80e5fbdd20cd57411af5c76076a15dced05ac6e6d9afa88cd7337bf9c869f6785ecc1c568ca593a99dfeec14e024c0cd7822"}},
 	}
 
 	var txSigantures TransactionSignatures
+	chain := flow.Localnet.Chain()
 	for _, test := range tests {
 		sigs := make([]models.TransactionSignature, len(test.inAddresses))
 		for i, a := range test.inAddresses {
@@ -71,7 +73,7 @@ func TestTransactionSignatures_ValidParse(t *testing.T) {
 			sigs[i].Address = a
 		}
 
-		err := txSigantures.Parse(sigs)
+		err := txSigantures.Parse(sigs, chain)
 		assert.NoError(t, err)
 
 		assert.Equal(t, len(txSigantures), len(sigs))

diff --git a/engine/access/rest/request/signatures.go b/engine/access/rest/request/signatures.go
@@ -14,8 +14,9 @@ func (s *TransactionSignature) Parse(
 	rawAddress string,
 	rawKeyIndex string,
 	rawSignature string,
+	chain flow.Chain,
 ) error {
-	address, err := ParseAddress(rawAddress)
+	address, err := ParseAddress(rawAddress, chain)
 	if err != nil {
 		return err
 	}
@@ -46,11 +47,11 @@ func (s TransactionSignature) Flow() flow.TransactionSignature {
 
 type TransactionSignatures []TransactionSignature
 
-func (t *TransactionSignatures) Parse(rawSigs []models.TransactionSignature) error {
+func (t *TransactionSignatures) Parse(rawSigs []models.TransactionSignature, chain flow.Chain) error {
 	signatures := make([]TransactionSignature, len(rawSigs))
 	for i, sig := range rawSigs {
 		var signature TransactionSignature
-		err := signature.Parse(sig.Address, sig.KeyIndex, sig.Signature)
+		err := signature.Parse(sig.Address, sig.KeyIndex, sig.Signature, chain)
 		if err != nil {
 			return err
 		}

diff --git a/engine/access/rest/request/transaction.go b/engine/access/rest/request/transaction.go
@@ -50,14 +50,14 @@ func (t *Transaction) Parse(raw io.Reader, chain flow.Chain) error {
 		return err
 	}
 
-	payer, err := ParseAddress(tx.Payer)
+	payer, err := ParseAddress(tx.Payer, chain)
 	if err != nil {
 		return fmt.Errorf("invalid payer: %w", err)
 	}
 
 	auths := make([]flow.Address, len(tx.Authorizers))
 	for i, auth := range tx.Authorizers {
-		a, err := ParseAddress(auth)
+		a, err := ParseAddress(auth, chain)
 		if err != nil {
 			return err
 		}
@@ -66,19 +66,19 @@ func (t *Transaction) Parse(raw io.Reader, chain flow.Chain) error {
 	}
 
 	var proposal ProposalKey
-	err = proposal.Parse(*tx.ProposalKey)
+	err = proposal.Parse(*tx.ProposalKey, chain)
 	if err != nil {
 		return err
 	}
 
 	var payloadSigs TransactionSignatures
-	err = payloadSigs.Parse(tx.PayloadSignatures)
+	err = payloadSigs.Parse(tx.PayloadSignatures, chain)
 	if err != nil {
 		return err
 	}
 
 	var envelopeSigs TransactionSignatures
-	err = envelopeSigs.Parse(tx.EnvelopeSignatures)
+	err = envelopeSigs.Parse(tx.EnvelopeSignatures, chain)
 	if err != nil {
 		return err
 	}

diff --git a/engine/common/rpc/convert/validate.go b/engine/common/rpc/convert/validate.go
@@ -26,6 +26,20 @@ func Address(rawAddress []byte, chain flow.Chain) (flow.Address, error) {
 	return address, nil
 }
 
+func HexToAddress(hexAddress string, chain flow.Chain) (flow.Address, error) {
+	if len(hexAddress) == 0 {
+		return flow.EmptyAddress, status.Error(codes.InvalidArgument, "address cannot be empty")
+	}
+
+	address := flow.HexToAddress(hexAddress)
+
+	if !chain.IsValid(address) {
+		return flow.EmptyAddress, status.Errorf(codes.InvalidArgument, "address %s is invalid for chain %s", address, chain)
+	}
+
+	return address, nil
+}
+
 func BlockID(blockID []byte) (flow.Identifier, error) {
 	if len(blockID) != flow.IdentifierLen {
 		return flow.ZeroID, status.Error(codes.InvalidArgument, "invalid block id")