Merge remote-tracking branch 'upstream/master'

Author: actions-user

api/fiat_rates_api.go

@@ -10,6 +10,9 @@ import (
 	"github.com/trezor/blockbook/common"
 )
 
+// MaxFiatRatesTimestamps limits batch fiat-rate lookups to bounded request work.
+const MaxFiatRatesTimestamps = 1000
+
 // removeEmpty removes empty strings from a slice.
 func removeEmpty(stringSlice []string) []string {
 	ret := make([]string, 0, len(stringSlice))
@@ -128,6 +131,9 @@ func (w *Worker) GetFiatRatesForTimestamps(timestamps []int64, currencies []stri
 	if len(timestamps) == 0 {
 		return nil, NewAPIError("No timestamps provided", true)
 	}
+	if len(timestamps) > MaxFiatRatesTimestamps {
+		return nil, NewAPIError(fmt.Sprintf("too many timestamps, max %d", MaxFiatRatesTimestamps), true)
+	}
 	vsCurrency := ""
 	currencies = removeEmpty(currencies)
 	if len(currencies) == 1 {

api/fiat_rates_api_test.go

@@ -3,6 +3,7 @@
 package api
 
 import (
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -191,6 +192,17 @@ func TestGetFiatRatesForTimestamps_EmptyInput(t *testing.T) {
 	}
 }
 
+func TestGetFiatRatesForTimestamps_LimitsInput(t *testing.T) {
+	w := &Worker{}
+	timestamps := make([]int64, MaxFiatRatesTimestamps+1)
+	_, err := w.GetFiatRatesForTimestamps(timestamps, []string{"usd"}, "")
+	apiErr := requireAPIError(t, err, true)
+	want := fmt.Sprintf("too many timestamps, max %d", MaxFiatRatesTimestamps)
+	if apiErr.Text != want {
+		t.Fatalf("unexpected error text: got %q, want %q", apiErr.Text, want)
+	}
+}
+
 func TestGetFiatRatesForTimestamps_LenMismatchReturnsNonPublicError(t *testing.T) {
 	w := &Worker{fiatRates: &fiat.FiatRates{}}
 	originalGetter := getTickersForTimestamps

api/xpub.go

@@ -22,6 +22,8 @@ const txInput = 1
 const txOutput = 2
 
 const xpubCacheExpirationSeconds = 3600
+const xpubCacheMaxEntries = 128
+const maxXpubAddressDerivations = (maxAddressesGap + 1) * 2
 
 var cachedXpubs map[string]xpubData
 var cachedXpubsMux sync.Mutex
@@ -85,18 +87,62 @@ func (w *Worker) initXpubCache() {
 }
 
 func (w *Worker) evictXpubCacheItems() {
+	now := time.Now().Unix()
 	cachedXpubsMux.Lock()
-	defer cachedXpubsMux.Unlock()
-	threshold := time.Now().Unix() - xpubCacheExpirationSeconds
+	count := evictXpubCacheItemsLocked(now)
+	cacheSize := len(cachedXpubs)
+	cachedXpubsMux.Unlock()
+
+	w.metrics.XPubCacheSize.Set(float64(cacheSize))
+	glog.Info("Evicted ", count, " items from xpub cache, cache size ", cacheSize)
+}
+
+func evictXpubCacheItemsLocked(now int64) int {
+	threshold := now - xpubCacheExpirationSeconds
 	count := 0
 	for k, v := range cachedXpubs {
 		if v.accessed < threshold {
 			delete(cachedXpubs, k)
 			count++
 		}
 	}
-	w.metrics.XPubCacheSize.Set(float64(len(cachedXpubs)))
-	glog.Info("Evicted ", count, " items from xpub cache, cache size ", len(cachedXpubs))
+	return count + trimXpubCacheItemsLocked()
+}
+
+func trimXpubCacheItemsLocked() int {
+	if len(cachedXpubs) <= xpubCacheMaxEntries {
+		return 0
+	}
+	type cacheEntry struct {
+		key      string
+		accessed int64
+	}
+	entries := make([]cacheEntry, 0, len(cachedXpubs))
+	for k, v := range cachedXpubs {
+		entries = append(entries, cacheEntry{key: k, accessed: v.accessed})
+	}
+	sort.Slice(entries, func(i, j int) bool {
+		if entries[i].accessed == entries[j].accessed {
+			return entries[i].key < entries[j].key
+		}
+		return entries[i].accessed < entries[j].accessed
+	})
+	count := len(cachedXpubs) - xpubCacheMaxEntries
+	for i := 0; i < count; i++ {
+		delete(cachedXpubs, entries[i].key)
+	}
+	return count
+}
+
+func validateXpubScanLimits(xd *bchain.XpubDescriptor, gap int) error {
+	if len(xd.ChangeIndexes) > bchain.MaxXpubChangeIndexes {
+		return errors.Errorf("Xpub descriptor change index count %d exceeds limit %d", len(xd.ChangeIndexes), bchain.MaxXpubChangeIndexes)
+	}
+	derivations := len(xd.ChangeIndexes) * gap
+	if derivations > maxXpubAddressDerivations {
+		return errors.Errorf("Xpub descriptor scan size %d exceeds limit %d", derivations, maxXpubAddressDerivations)
+	}
+	return nil
 }
 
 func (w *Worker) xpubGetAddressTxids(addrDesc bchain.AddressDescriptor, mempool bool, fromHeight, toHeight uint32, maxResults int) ([]xpubTxid, bool, error) {
@@ -203,7 +249,10 @@ func (w *Worker) xpubDerivedAddressBalance(data *xpubData, ad *xpubAddress) (boo
 	return false, nil
 }
 
-func (w *Worker) xpubScanAddresses(xd *bchain.XpubDescriptor, data *xpubData, addresses []xpubAddress, gap int, change uint32, minDerivedIndex int, fork bool) (int, []xpubAddress, error) {
+func (w *Worker) xpubScanAddresses(xd *bchain.XpubDescriptor, data *xpubData, addresses []xpubAddress, gap int, change uint32, minDerivedIndex int, fork bool, derivedBefore int) (int, []xpubAddress, error) {
+	if total := derivedBefore + len(addresses); total > maxXpubAddressDerivations {
+		return 0, nil, errors.Errorf("Xpub descriptor scan size %d exceeds limit %d", total, maxXpubAddressDerivations)
+	}
 	// rescan known addresses
 	lastUsed := 0
 	for i := range addresses {
@@ -231,6 +280,9 @@ func (w *Worker) xpubScanAddresses(xd *bchain.XpubDescriptor, data *xpubData, ad
 		if to < minDerivedIndex {
 			to = minDerivedIndex
 		}
+		if total := derivedBefore + to; total > maxXpubAddressDerivations {
+			return 0, nil, errors.Errorf("Xpub descriptor scan size %d exceeds limit %d", total, maxXpubAddressDerivations)
+		}
 		descriptors, err := w.chainParser.DeriveAddressDescriptorsFromTo(xd, change, uint32(from), uint32(to))
 		if err != nil {
 			return 0, nil, err
@@ -325,6 +377,9 @@ func (w *Worker) getXpubData(xd *bchain.XpubDescriptor, page int, txsOnPage int,
 	}
 	// gap is increased one as there must be gap of empty addresses before the derivation is stopped
 	gap++
+	if err := validateXpubScanLimits(xd, gap); err != nil {
+		return nil, 0, false, err
+	}
 	var processedHash string
 	cachedXpubsMux.Lock()
 	data, inCache := cachedXpubs[xd.XpubDescriptor]
@@ -366,11 +421,13 @@ func (w *Worker) getXpubData(xd *bchain.XpubDescriptor, page int, txsOnPage int,
 			data.sentSat = *new(big.Int)
 			data.txCountEstimate = 0
 			var minDerivedIndex int
+			totalDerived := 0
 			for i, change := range xd.ChangeIndexes {
-				minDerivedIndex, data.addresses[i], err = w.xpubScanAddresses(xd, &data, data.addresses[i], gap, change, minDerivedIndex, fork)
+				minDerivedIndex, data.addresses[i], err = w.xpubScanAddresses(xd, &data, data.addresses[i], gap, change, minDerivedIndex, fork, totalDerived)
 				if err != nil {
 					return nil, 0, inCache, err
 				}
+				totalDerived += len(data.addresses[i])
 			}
 		}
 		if option >= AccountDetailsTxidHistory {
@@ -385,8 +442,14 @@ func (w *Worker) getXpubData(xd *bchain.XpubDescriptor, page int, txsOnPage int,
 	}
 	data.accessed = time.Now().Unix()
 	cachedXpubsMux.Lock()
+	if cachedXpubs == nil {
+		cachedXpubs = make(map[string]xpubData)
+	}
 	cachedXpubs[xd.XpubDescriptor] = data
+	trimXpubCacheItemsLocked()
+	cacheSize := len(cachedXpubs)
 	cachedXpubsMux.Unlock()
+	w.metrics.XPubCacheSize.Set(float64(cacheSize))
 	return &data, bestheight, inCache, nil
 }
 

api/xpub_test.go

@@ -0,0 +1,54 @@
+//go:build unittest
+
+package api
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/trezor/blockbook/bchain"
+)
+
+func TestValidateXpubScanLimits(t *testing.T) {
+	if err := validateXpubScanLimits(&bchain.XpubDescriptor{ChangeIndexes: []uint32{0, 1}}, maxAddressesGap+1); err != nil {
+		t.Fatalf("expected default change indexes at max gap to pass, got %v", err)
+	}
+
+	changes := make([]uint32, bchain.MaxXpubChangeIndexes+1)
+	if err := validateXpubScanLimits(&bchain.XpubDescriptor{ChangeIndexes: changes}, defaultAddressesGap+1); err == nil {
+		t.Fatal("expected change index count above limit to fail")
+	}
+
+	changes = make([]uint32, 3)
+	if err := validateXpubScanLimits(&bchain.XpubDescriptor{ChangeIndexes: changes}, maxAddressesGap+1); err == nil {
+		t.Fatal("expected scan size above limit to fail")
+	}
+}
+
+func TestTrimXpubCacheItemsLocked(t *testing.T) {
+	cachedXpubsMux.Lock()
+	defer cachedXpubsMux.Unlock()
+
+	originalCache := cachedXpubs
+	defer func() {
+		cachedXpubs = originalCache
+	}()
+
+	cachedXpubs = make(map[string]xpubData, xpubCacheMaxEntries+2)
+	for i := 0; i < xpubCacheMaxEntries+2; i++ {
+		cachedXpubs[fmt.Sprintf("xpub-%03d", i)] = xpubData{accessed: int64(i)}
+	}
+
+	if got := trimXpubCacheItemsLocked(); got != 2 {
+		t.Fatalf("trimXpubCacheItemsLocked() evicted %d entries, want 2", got)
+	}
+	if got := len(cachedXpubs); got != xpubCacheMaxEntries {
+		t.Fatalf("cachedXpubs length = %d, want %d", got, xpubCacheMaxEntries)
+	}
+	if _, ok := cachedXpubs["xpub-000"]; ok {
+		t.Fatal("oldest cache entry was not evicted")
+	}
+	if _, ok := cachedXpubs["xpub-001"]; ok {
+		t.Fatal("second oldest cache entry was not evicted")
+	}
+}

bchain/coins/btc/bitcoinlikeparser.go

@@ -441,7 +441,11 @@ var (
 )
 
 func init() {
-	xpubDesriptorRegex, _ = regexp.Compile(`^(?P<type>(sh\(wpkh|wpkh|pk|pkh|wpkh|wsh|tr))\((\[\w+/(?P<bip>\d+)['h]/\d+['h]?/\d+['h]?\])?(?P<xpub>\w+)(/(({(?P<changelist1>\d+(,\d+)*)})|(<(?P<changelist2>\d+(;\d+)*)>)|(?P<change>\d+))/\*)?\)+`)
+	var err error
+	xpubDesriptorRegex, err = regexp.Compile(`^(?P<type>(sh\(wpkh|wpkh|pk|pkh|wpkh|wsh|tr))\((\[\w+/(?P<bip>\d+)['h]/\d+['h]?/\d+['h]?\])?(?P<xpub>\w+)(/(({(?P<changelist1>\d+(,\d+)*)})|(<(?P<changelist2>\d+(;\d+)*)>)|(?P<change>\d+))/\*)?\)+`)
+	if err != nil {
+		panic(errors.Annotate(err, "Invalid bitcoinparser xpubDesriptorRegex"))
+	}
 	typeSubexpIndex = xpubDesriptorRegex.SubexpIndex("type")
 	bipSubexpIndex = xpubDesriptorRegex.SubexpIndex("bip")
 	xpubSubexpIndex = xpubDesriptorRegex.SubexpIndex("xpub")
@@ -502,6 +506,9 @@ func (p *BitcoinLikeParser) ParseXpub(xpub string) (*bchain.XpubDescriptor, erro
 				if len(changes) == 0 {
 					return nil, errors.New("Invalid xpub descriptor, cannot parse change")
 				}
+				if len(changes) > bchain.MaxXpubChangeIndexes {
+					return nil, errors.Errorf("Xpub descriptor change index count exceeds limit %d", bchain.MaxXpubChangeIndexes)
+				}
 				descriptor.ChangeIndexes = make([]uint32, len(changes))
 				for i, ch := range changes {
 					change, err := strconv.ParseUint(ch, 10, 32)

bchain/coins/btc/bitcoinparser_test.go

@@ -7,6 +7,8 @@ import (
 	"math/big"
 	"os"
 	"reflect"
+	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/martinboehm/btcutil/chaincfg"
@@ -19,6 +21,22 @@ func TestMain(m *testing.M) {
 	os.Exit(c)
 }
 
+func testChangeList(count int, separator string) string {
+	changes := make([]string, count)
+	for i := range changes {
+		changes[i] = strconv.Itoa(i)
+	}
+	return strings.Join(changes, separator)
+}
+
+func testChangeIndexes(count int) []uint32 {
+	indexes := make([]uint32, count)
+	for i := range indexes {
+		indexes[i] = uint32(i)
+	}
+	return indexes
+}
+
 func TestGetAddrDescFromAddress(t *testing.T) {
 	type args struct {
 		address string
@@ -781,11 +799,12 @@ func TestParseXpubDescriptors(t *testing.T) {
 	btcMainParser := NewBitcoinParser(GetChainParams("main"), &Configuration{XPubMagic: 76067358, XPubMagicSegwitP2sh: 77429938, XPubMagicSegwitNative: 78792518})
 	btcTestnetParser := NewBitcoinParser(GetChainParams("test"), &Configuration{XPubMagic: 70617039, XPubMagicSegwitP2sh: 71979618, XPubMagicSegwitNative: 73342198})
 	tests := []struct {
-		name    string
-		xpub    string
-		parser  *BitcoinParser
-		want    *bchain.XpubDescriptor
-		wantErr bool
+		name            string
+		xpub            string
+		parser          *BitcoinParser
+		want            *bchain.XpubDescriptor
+		wantErr         bool
+		wantErrContains string
 	}{
 		{
 			name:   "tpub",
@@ -847,6 +866,32 @@ func TestParseXpubDescriptors(t *testing.T) {
 				ChangeIndexes:  []uint32{0, 1, 2},
 			},
 		},
+		{
+			name:   "tr([5c9e228d/86'/1'/0']tpubD/{max changes}/*)#4rqwxvej",
+			xpub:   "tr([5c9e228d/86'/1'/0']tpubDC88gkaZi5HvJGxGDNLADkvtdpni3mLmx6vr2KnXmWMG8zfkBRggsxHVBkUpgcwPe2KKpkyvTJCdXHb1UHEWE64vczyyPQfHr1skBcsRedN/{" + testChangeList(bchain.MaxXpubChangeIndexes, ",") + "}/*)#4rqwxvej",
+			parser: btcTestnetParser,
+			want: &bchain.XpubDescriptor{
+				XpubDescriptor: "tr([5c9e228d/86'/1'/0']tpubDC88gkaZi5HvJGxGDNLADkvtdpni3mLmx6vr2KnXmWMG8zfkBRggsxHVBkUpgcwPe2KKpkyvTJCdXHb1UHEWE64vczyyPQfHr1skBcsRedN/{" + testChangeList(bchain.MaxXpubChangeIndexes, ",") + "}/*)#4rqwxvej",
+				Xpub:           "tpubDC88gkaZi5HvJGxGDNLADkvtdpni3mLmx6vr2KnXmWMG8zfkBRggsxHVBkUpgcwPe2KKpkyvTJCdXHb1UHEWE64vczyyPQfHr1skBcsRedN",
+				Type:           bchain.P2TR,
+				Bip:            "86",
+				ChangeIndexes:  testChangeIndexes(bchain.MaxXpubChangeIndexes),
+			},
+		},
+		{
+			name:            "tr([5c9e228d/86'/1'/0']tpubD/{too many changes}/*)#4rqwxvej error - change list limit",
+			xpub:            "tr([5c9e228d/86'/1'/0']tpubDC88gkaZi5HvJGxGDNLADkvtdpni3mLmx6vr2KnXmWMG8zfkBRggsxHVBkUpgcwPe2KKpkyvTJCdXHb1UHEWE64vczyyPQfHr1skBcsRedN/{" + testChangeList(bchain.MaxXpubChangeIndexes+1, ",") + "}/*)#4rqwxvej",
+			parser:          btcTestnetParser,
+			wantErr:         true,
+			wantErrContains: "change index count exceeds limit",
+		},
+		{
+			name:            "tr([5c9e228d/86'/1'/0']tpubD/<too many changes>/*)#4rqwxvej error - change list limit",
+			xpub:            "tr([5c9e228d/86'/1'/0']tpubDC88gkaZi5HvJGxGDNLADkvtdpni3mLmx6vr2KnXmWMG8zfkBRggsxHVBkUpgcwPe2KKpkyvTJCdXHb1UHEWE64vczyyPQfHr1skBcsRedN/<" + testChangeList(bchain.MaxXpubChangeIndexes+1, ";") + ">/*)#4rqwxvej",
+			parser:          btcTestnetParser,
+			wantErr:         true,
+			wantErrContains: "change index count exceeds limit",
+		},
 		{
 			name:   "tr([5c9e228d/86'/1'/0']tpubD/3/*)#4rqwxvej",
 			xpub:   "tr([5c9e228d/86'/1'/0']tpubDC88gkaZi5HvJGxGDNLADkvtdpni3mLmx6vr2KnXmWMG8zfkBRggsxHVBkUpgcwPe2KKpkyvTJCdXHb1UHEWE64vczyyPQfHr1skBcsRedN/3/*)#4rqwxvej",
@@ -981,6 +1026,9 @@ func TestParseXpubDescriptors(t *testing.T) {
 				t.Errorf("ParseXpub() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
+			if err != nil && tt.wantErrContains != "" && !strings.Contains(err.Error(), tt.wantErrContains) {
+				t.Errorf("ParseXpub() error = %v, want error containing %q", err, tt.wantErrContains)
+			}
 			if err == nil {
 				if got.ExtKey == nil {
 					t.Errorf("ParseXpub() got nil ExtKey")

bchain/types.go

@@ -267,6 +267,10 @@ const (
 	P2TR
 )
 
+// MaxXpubChangeIndexes limits how many change branches one xpub descriptor can
+// expand during account scans.
+const MaxXpubChangeIndexes = 10
+
 // XpubDescriptor contains parsed data from xpub descriptor
 type XpubDescriptor struct {
 	XpubDescriptor string      `ts_doc:"Full descriptor string including xpub and script type."`

server/public.go

@@ -1699,6 +1699,22 @@ func (s *PublicServer) apiTickers(r *http.Request, apiVersion int) (interface{},
 	return result, nil
 }
 
+func countCommaSeparatedValues(s string, limit int) int {
+	if s == "" {
+		return 0
+	}
+	count := 1
+	for i := 0; i < len(s); i++ {
+		if s[i] == ',' {
+			count++
+			if count > limit {
+				return count
+			}
+		}
+	}
+	return count
+}
+
 // apiMultiTickers returns FiatRates ticker prices for the specified comma separated list of timestamps.
 func (s *PublicServer) apiMultiTickers(r *http.Request, apiVersion int) (interface{}, error) {
 	var result []api.FiatTicker
@@ -1713,6 +1729,9 @@ func (s *PublicServer) apiMultiTickers(r *http.Request, apiVersion int) (interfa
 	if timestampString := r.URL.Query().Get("timestamp"); timestampString != "" {
 		// Get tickers for specified timestamp
 		s.metrics.ExplorerViews.With(common.Labels{"action": "api-multi-tickers-date"}).Inc()
+		if countCommaSeparatedValues(timestampString, api.MaxFiatRatesTimestamps) > api.MaxFiatRatesTimestamps {
+			return nil, api.NewAPIError(fmt.Sprintf("too many timestamps, max %d", api.MaxFiatRatesTimestamps), true)
+		}
 		timestamps := strings.Split(timestampString, ",")
 		t := make([]int64, len(timestamps))
 		for i := range timestamps {