eth/filter: bugfix which can cause a nil pointer crash when parsing filter arguments

2 files changed, 234 insertions, 38 deletions

diff --git a/eth/filters/api.go b/eth/filters/api.go
index d9bd4d4b7..7278e20b9 100644
--- a/eth/filters/api.go
+++ b/eth/filters/api.go
@@ -233,6 +233,7 @@ func (s *PublicFilterAPI) newLogFilter(earliest, latest int64, addresses []commo
 	return id, nil
 }
 
+// Logs creates a subscription that fires for all new log that match the given filter criteria.
 func (s *PublicFilterAPI) Logs(ctx context.Context, args NewFilterArgs) (rpc.Subscription, error) {
 	notifier, supported := rpc.NotifierFromContext(ctx)
 	if !supported {
@@ -291,12 +292,13 @@ type NewFilterArgs struct {
 	Topics    [][]common.Hash
 }
 
+// UnmarshalJSON sets *args fields with given data.
 func (args *NewFilterArgs) UnmarshalJSON(data []byte) error {
 	type input struct {
 		From      *rpc.BlockNumber `json:"fromBlock"`
 		ToBlock   *rpc.BlockNumber `json:"toBlock"`
 		Addresses interface{}      `json:"address"`
-		Topics    interface{}      `json:"topics"`
+		Topics    []interface{}    `json:"topics"`
 	}
 
 	var raw input
@@ -321,7 +323,6 @@ func (args *NewFilterArgs) UnmarshalJSON(data []byte) error {
 	if raw.Addresses != nil {
 		// raw.Address can contain a single address or an array of addresses
 		var addresses []common.Address
-
 		if strAddrs, ok := raw.Addresses.([]interface{}); ok {
 			for i, addr := range strAddrs {
 				if strAddr, ok := addr.(string); ok {
@@ -352,56 +353,53 @@ func (args *NewFilterArgs) UnmarshalJSON(data []byte) error {
 		args.Addresses = addresses
 	}
 
+	// helper function which parses a string to a topic hash
 	topicConverter := func(raw string) (common.Hash, error) {
 		if len(raw) == 0 {
 			return common.Hash{}, nil
 		}
-
 		if len(raw) >= 2 && raw[0] == '0' && (raw[1] == 'x' || raw[1] == 'X') {
 			raw = raw[2:]
 		}
-
+		if len(raw) != 2 * common.HashLength {
+			return common.Hash{}, errors.New("invalid topic(s)")
+		}
 		if decAddr, err := hex.DecodeString(raw); err == nil {
 			return common.BytesToHash(decAddr), nil
 		}
-
-		return common.Hash{}, errors.New("invalid topic given")
-	}
-
-	// topics is an array consisting of strings or arrays of strings
-	if raw.Topics != nil {
-		topics, ok := raw.Topics.([]interface{})
-		if ok {
-			parsedTopics := make([][]common.Hash, len(topics))
-			for i, topic := range topics {
-				if topic == nil {
-					parsedTopics[i] = []common.Hash{common.StringToHash("")}
-				} else if strTopic, ok := topic.(string); ok {
-					if t, err := topicConverter(strTopic); err != nil {
-						return fmt.Errorf("invalid topic on index %d", i)
-					} else {
-						parsedTopics[i] = []common.Hash{t}
-					}
-				} else if arrTopic, ok := topic.([]interface{}); ok {
-					parsedTopics[i] = make([]common.Hash, len(arrTopic))
-					for j := 0; j < len(parsedTopics[i]); i++ {
-						if arrTopic[j] == nil {
-							parsedTopics[i][j] = common.StringToHash("")
-						} else if str, ok := arrTopic[j].(string); ok {
-							if t, err := topicConverter(str); err != nil {
-								return fmt.Errorf("invalid topic on index %d", i)
-							} else {
-								parsedTopics[i] = []common.Hash{t}
-							}
-						} else {
-							return fmt.Errorf("topic[%d][%d] not a string", i, j)
+		return common.Hash{}, errors.New("invalid topic(s)")
+	}
+
+	// topics is an array consisting of strings and/or arrays of strings.
+	// JSON null values are converted to common.Hash{} and ignored by the filter manager.
+	if len(raw.Topics) > 0 {
+		args.Topics = make([][]common.Hash, len(raw.Topics))
+		for i, t := range raw.Topics {
+			if t == nil { // ignore topic when matching logs
+				args.Topics[i] = []common.Hash{common.Hash{}}
+			} else if topic, ok := t.(string); ok { // match specific topic
+				top, err := topicConverter(topic)
+				if err != nil {
+					return err
+				}
+				args.Topics[i] = []common.Hash{top}
+			} else if topics, ok := t.([]interface{}); ok { // or case e.g. [null, "topic0", "topic1"]
+				for _, rawTopic := range topics {
+					if rawTopic == nil {
+						args.Topics[i] = append(args.Topics[i], common.Hash{})
+					} else if topic, ok := rawTopic.(string); ok {
+						parsed, err := topicConverter(topic)
+						if err != nil {
+							return err
 						}
+						args.Topics[i] = append(args.Topics[i], parsed)
+					} else {
+						return fmt.Errorf("invalid topic(s)")
 					}
-				} else {
-					return fmt.Errorf("topic[%d] invalid", i)
 				}
+			} else {
+				return fmt.Errorf("invalid topic(s)")
 			}
-			args.Topics = parsedTopics
 		}
 	}
 
diff --git a/eth/filters/api_test.go b/eth/filters/api_test.go
new file mode 100644
index 000000000..9e8edc241
--- /dev/null
+++ b/eth/filters/api_test.go
@@ -0,0 +1,198 @@
+// Copyright 2016 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+package filters_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/eth/filters"
+	"github.com/ethereum/go-ethereum/rpc"
+)
+
+func TestUnmarshalJSONNewFilterArgs(t *testing.T) {
+	var (
+		fromBlock rpc.BlockNumber = 0x123435
+		toBlock   rpc.BlockNumber = 0xabcdef
+		address0                  = common.StringToAddress("70c87d191324e6712a591f304b4eedef6ad9bb9d")
+		address1                  = common.StringToAddress("9b2055d370f73ec7d8a03e965129118dc8f5bf83")
+		topic0                    = common.HexToHash("3ac225168df54212a25c1c01fd35bebfea408fdac2e31ddd6f80a4bbf9a5f1ca")
+		topic1                    = common.HexToHash("9084a792d2f8b16a62b882fd56f7860c07bf5fa91dd8a2ae7e809e5180fef0b3")
+		topic2                    = common.HexToHash("6ccae1c4af4152f460ff510e573399795dfab5dcf1fa60d1f33ac8fdc1e480ce")
+		nullTopic                 = common.Hash{}
+	)
+
+	// default values
+	var test0 filters.NewFilterArgs
+	if err := json.Unmarshal([]byte("{}"), &test0); err != nil {
+		t.Fatal(err)
+	}
+	if test0.FromBlock != rpc.LatestBlockNumber {
+		t.Fatalf("expected %d, got %d", rpc.LatestBlockNumber, test0.FromBlock)
+	}
+	if test0.ToBlock != rpc.LatestBlockNumber {
+		t.Fatalf("expected %d, got %d", rpc.LatestBlockNumber, test0.ToBlock)
+	}
+	if len(test0.Addresses) != 0 {
+		t.Fatalf("expected 0 addresses, got %d", len(test0.Addresses))
+	}
+	if len(test0.Topics) != 0 {
+		t.Fatalf("expected 0 topics, got %d topics", len(test0.Topics))
+	}
+
+	// from, to block number
+	var test1 filters.NewFilterArgs
+	vector := fmt.Sprintf(`{"fromBlock":"0x%x","toBlock":"0x%x"}`, fromBlock, toBlock)
+	if err := json.Unmarshal([]byte(vector), &test1); err != nil {
+		t.Fatal(err)
+	}
+	if test1.FromBlock != fromBlock {
+		t.Fatalf("expected FromBlock %d, got %d", fromBlock, test1.FromBlock)
+	}
+	if test1.ToBlock != toBlock {
+		t.Fatalf("expected ToBlock %d, got %d", toBlock, test1.ToBlock)
+	}
+
+	// single address
+	var test2 filters.NewFilterArgs
+	vector = fmt.Sprintf(`{"address": "%s"}`, address0.Hex())
+	if err := json.Unmarshal([]byte(vector), &test2); err != nil {
+		t.Fatal(err)
+	}
+	if len(test2.Addresses) != 1 {
+		t.Fatalf("expected 1 address, got %d address(es)", len(test2.Addresses))
+	}
+	if test2.Addresses[0] != address0 {
+		t.Fatalf("expected address %x, got %x", address0, test2.Addresses[0])
+	}
+
+	// multiple address
+	var test3 filters.NewFilterArgs
+	vector = fmt.Sprintf(`{"address": ["%s", "%s"]}`, address0.Hex(), address1.Hex())
+	if err := json.Unmarshal([]byte(vector), &test3); err != nil {
+		t.Fatal(err)
+	}
+	if len(test3.Addresses) != 2 {
+		t.Fatalf("expected 2 addresses, got %d address(es)", len(test3.Addresses))
+	}
+	if test3.Addresses[0] != address0 {
+		t.Fatalf("expected address %x, got %x", address0, test3.Addresses[0])
+	}
+	if test3.Addresses[1] != address1 {
+		t.Fatalf("expected address %x, got %x", address1, test3.Addresses[1])
+	}
+
+	// single topic
+	var test4 filters.NewFilterArgs
+	vector = fmt.Sprintf(`{"topics": ["%s"]}`, topic0.Hex())
+	if err := json.Unmarshal([]byte(vector), &test4); err != nil {
+		t.Fatal(err)
+	}
+	if len(test4.Topics) != 1 {
+		t.Fatalf("expected 1 topic, got %d", len(test4.Topics))
+	}
+	if len(test4.Topics[0]) != 1 {
+		t.Fatalf("expected len(topics[0]) to be 1, got %d", len(test4.Topics[0]))
+	}
+	if test4.Topics[0][0] != topic0 {
+		t.Fatalf("got %x, expected %x", test4.Topics[0][0], topic0)
+	}
+
+	// test multiple "AND" topics
+	var test5 filters.NewFilterArgs
+	vector = fmt.Sprintf(`{"topics": ["%s", "%s"]}`, topic0.Hex(), topic1.Hex())
+	if err := json.Unmarshal([]byte(vector), &test5); err != nil {
+		t.Fatal(err)
+	}
+	if len(test5.Topics) != 2 {
+		t.Fatalf("expected 2 topics, got %d", len(test5.Topics))
+	}
+	if len(test5.Topics[0]) != 1 {
+		t.Fatalf("expected 1 topic, got %d", len(test5.Topics[0]))
+	}
+	if test5.Topics[0][0] != topic0 {
+		t.Fatalf("got %x, expected %x", test5.Topics[0][0], topic0)
+	}
+	if len(test5.Topics[1]) != 1 {
+		t.Fatalf("expected 1 topic, got %d", len(test5.Topics[1]))
+	}
+	if test5.Topics[1][0] != topic1 {
+		t.Fatalf("got %x, expected %x", test5.Topics[1][0], topic1)
+	}
+
+	// test optional topic
+	var test6 filters.NewFilterArgs
+	vector = fmt.Sprintf(`{"topics": ["%s", null, "%s"]}`, topic0.Hex(), topic2.Hex())
+	if err := json.Unmarshal([]byte(vector), &test6); err != nil {
+		t.Fatal(err)
+	}
+	if len(test6.Topics) != 3 {
+		t.Fatalf("expected 3 topics, got %d", len(test6.Topics))
+	}
+	if len(test6.Topics[0]) != 1 {
+		t.Fatalf("expected 1 topic, got %d", len(test6.Topics[0]))
+	}
+	if test6.Topics[0][0] != topic0 {
+		t.Fatalf("got %x, expected %x", test6.Topics[0][0], topic0)
+	}
+	if len(test6.Topics[1]) != 1 {
+		t.Fatalf("expected 1 topic, got %d", len(test6.Topics[1]))
+	}
+	if test6.Topics[1][0] != nullTopic {
+		t.Fatalf("got %x, expected empty hash", test6.Topics[1][0])
+	}
+	if len(test6.Topics[2]) != 1 {
+		t.Fatalf("expected 1 topic, got %d", len(test6.Topics[2]))
+	}
+	if test6.Topics[2][0] != topic2 {
+		t.Fatalf("got %x, expected %x", test6.Topics[2][0], topic2)
+	}
+
+	// test OR topics
+	var test7 filters.NewFilterArgs
+	vector = fmt.Sprintf(`{"topics": [["%s", "%s"], null, ["%s", null]]}`, topic0.Hex(), topic1.Hex(), topic2.Hex())
+	if err := json.Unmarshal([]byte(vector), &test7); err != nil {
+		t.Fatal(err)
+	}
+	if len(test7.Topics) != 3 {
+		t.Fatalf("expected 3 topics, got %d topics", len(test7.Topics))
+	}
+	if len(test7.Topics[0]) != 2 {
+		t.Fatalf("expected 2 topics, got %d topics", len(test7.Topics[0]))
+	}
+	if test7.Topics[0][0] != topic0 || test7.Topics[0][1] != topic1 {
+		t.Fatalf("invalid topics expected [%x,%x], got [%x,%x]",
+			topic0, topic1, test7.Topics[0][0], test7.Topics[0][1],
+		)
+	}
+	if len(test7.Topics[1]) != 1 {
+		t.Fatalf("expected 1 topic, got %d topics", len(test7.Topics[1]))
+	}
+	if test7.Topics[1][0] != nullTopic {
+		t.Fatalf("expected empty hash, got %x", test7.Topics[1][0])
+	}
+	if len(test7.Topics[2]) != 2 {
+		t.Fatalf("expected 2 topics, got %d topics", len(test7.Topics[2]))
+	}
+	if test7.Topics[2][0] != topic2 || test7.Topics[2][1] != nullTopic {
+		t.Fatalf("invalid topics expected [%x,%x], got [%x,%x]",
+			topic2, nullTopic, test7.Topics[2][0], test7.Topics[2][1],
+		)
+	}
+}