Harden non-sensitive validation paths

.ubsignore

@@ -0,0 +1,4 @@
+# Deploy scripts use console.log intentionally for progress output and
+# readFileSync/writeFileSync appropriately for synchronous deploy workflows.
+# These are not production application code; UBS warnings are false positives.
+solidity/ecdsa/deploy/16_initialize_allowlist_weights.ts

pkg/bitcoin/transaction_builder.go

@@ -148,7 +148,18 @@ func (tb *TransactionBuilder) getScript(
 		)
 	}
 
-	return transaction.Outputs[utxo.Outpoint.OutputIndex].PublicKeyScript, nil
+	outputIndex := utxo.Outpoint.OutputIndex
+	if outputIndex >= uint32(len(transaction.Outputs)) {
+		return nil, fmt.Errorf(
+			"output index [%d] out of range for transaction [%s] "+
+				"with [%d] outputs",
+			outputIndex,
+			hash.Hex(InternalByteOrder),
+			len(transaction.Outputs),
+		)
+	}
+
+	return transaction.Outputs[outputIndex].PublicKeyScript, nil
 }
 
 // AddOutput adds a new transaction's output.

pkg/bitcoin/transaction_builder_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"math/big"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/keep-network/keep-core/internal/testutils"
@@ -110,6 +111,35 @@ func TestTransactionBuilder_AddPublicKeyHashInput(t *testing.T) {
 	}
 }
 
+func TestTransactionBuilder_AddInputReturnsErrorForOutOfRangeOutputIndex(
+	t *testing.T,
+) {
+	localChain := newLocalChain()
+	builder := NewTransactionBuilder(localChain)
+
+	inputTransaction := transactionFrom(
+		t,
+		"01000000012d4e0b1ef0bf21eed32f6e2f11353b78534dcf21852d506f6f53b64bb5c6b4c500000000c84730440220590e998a5c28965fd442e700445a60c494124fdbb8aa39cc20c04f2aedadb1a602206acb2f852cd7adea65fe9209024e18d2d6ccac0b1e45c61d80c9bcd62f3e5a12012103989d253b17a6a0f41838b84ff0d20e8898f9d7b1a98f2564da4cc29dcf8581d94c5c14934b98637ca318a4d6e7ca6ffd1690b8e77df6377508f9f0c90d000395237576a9148db50eb52063ea9d98b3eac91489a90f738986f68763ac6776a914e257eccafbc07c381642ce6e7e55120fb077fbed880448f2b262b175ac68ffffffff0110400000000000001976a9148db50eb52063ea9d98b3eac91489a90f738986f688ac00000000",
+	)
+	if err := localChain.addTransaction(inputTransaction); err != nil {
+		t.Fatal(err)
+	}
+
+	err := builder.AddPublicKeyHashInput(&UnspentTransactionOutput{
+		Outpoint: &TransactionOutpoint{
+			TransactionHash: inputTransaction.Hash(),
+			OutputIndex:     3,
+		},
+		Value: 16400,
+	})
+	if err == nil {
+		t.Fatal("expected out-of-range output index error")
+	}
+	if !strings.Contains(err.Error(), "output index [3] out of range") {
+		t.Fatalf("unexpected error: [%v]", err)
+	}
+}
+
 func TestTransactionBuilder_AddScriptHashInput(t *testing.T) {
 	var tests = map[string]struct {
 		inputTransactionHex string

pkg/firewall/firewall.go

@@ -52,11 +52,6 @@ func (al *AllowList) Contains(operatorPublicKey *operator.PublicKey) bool {
 var EmptyAllowList = NewAllowList([]*operator.PublicKey{})
 
 const (
-	// PositiveIsRecognizedCachePeriod is the time period the cache maintains
-	// the positive result of the last `IsRecognized` checks.
-	// We use the cache to minimize calls to the on-chain client.
-	PositiveIsRecognizedCachePeriod = 12 * time.Hour
-
 	// NegativeIsRecognizedCachePeriod is the time period the cache maintains
 	// the negative result of the last `IsRecognized` checks.
 	// We use the cache to minimize calls to the on-chain client.
@@ -74,24 +69,24 @@ func AnyApplicationPolicy(
 	return &anyApplicationPolicy{
 		applications:        applications,
 		allowList:           allowList,
-		positiveResultCache: cache.NewTimeCache(PositiveIsRecognizedCachePeriod),
 		negativeResultCache: cache.NewTimeCache(NegativeIsRecognizedCachePeriod),
 	}
 }
 
 type anyApplicationPolicy struct {
 	applications        []Application
 	allowList           *AllowList
-	positiveResultCache *cache.TimeCache
 	negativeResultCache *cache.TimeCache
 }
 
 // Validate checks whether the given operator meets the conditions to join
 // the network. The operator can join the network if it is an allowlisted node
 // or it is a non-allowlisted node but it is recognized as eligible by any of
 // the applications. Nil is returned on a successful validation, error otherwise.
-// Due to performance reasons, the results of validations for non-allowlisted
-// nodes are stored in a cache for a certain amount of time.
+// Due to performance reasons, negative validation results for non-allowlisted
+// nodes are stored in a cache for a certain amount of time. Positive results
+// are intentionally not cached so that operator revocations take effect on the
+// next Validate call rather than after a cache TTL.
 func (aap *anyApplicationPolicy) Validate(
 	remotePeerPublicKey *operator.PublicKey,
 ) error {
@@ -100,23 +95,17 @@ func (aap *anyApplicationPolicy) Validate(
 		return nil
 	}
 
-	// First, check in the in-memory time caches to minimize hits to the ETH client.
-	// If the Keep client with the given chain address is in the positive result
-	// cache it means it has been recognized when the last `IsRecognized` was
-	// executed and caching period has not elapsed yet. Similarly, if the client
-	// is in the negative result cache it means it hasn't been recognized.
+	// First, check in the in-memory time cache to minimize hits to the ETH client.
+	// If the client is in the negative result cache it means it hasn't been
+	// recognized when the last `IsRecognized` was executed and caching period
+	// has not elapsed yet.
 	//
 	// If the caching period elapsed, cache checks will return false and we
 	// have to ask the chain about the current status.
-	aap.positiveResultCache.Sweep()
 	aap.negativeResultCache.Sweep()
 
 	remotePeerPublicKeyHex := remotePeerPublicKey.String()
 
-	if aap.positiveResultCache.Has(remotePeerPublicKeyHex) {
-		return nil
-	}
-
 	if aap.negativeResultCache.Has(remotePeerPublicKeyHex) {
 		return errNotRecognized
 	}
@@ -143,9 +132,5 @@ func (aap *anyApplicationPolicy) Validate(
 		return errNotRecognized
 	}
 
-	// Add this address to the positive result cache.
-	// `IsRecognized` will not be called again for the entire caching period.
-	aap.positiveResultCache.Add(remotePeerPublicKeyHex)
-
 	return nil
 }

pkg/firewall/firewall_test.go

@@ -17,7 +17,6 @@ func TestValidate_PeerNotRecognized_NoApplications(t *testing.T) {
 	policy := &anyApplicationPolicy{
 		applications:        []Application{},
 		allowList:           EmptyAllowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 
@@ -45,7 +44,6 @@ func TestValidate_PeerNotRecognized_MultipleApplications(t *testing.T) {
 			newMockApplication(),
 			newMockApplication()},
 		allowList:           EmptyAllowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 
@@ -72,7 +70,6 @@ func TestValidate_PeerRecognized_FirstApplicationRecognizes(t *testing.T) {
 			application,
 			newMockApplication()},
 		allowList:           EmptyAllowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 
@@ -101,7 +98,6 @@ func TestValidate_PeerRecognized_SecondApplicationRecognizes(t *testing.T) {
 			newMockApplication(),
 			application},
 		allowList:           EmptyAllowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 
@@ -140,15 +136,14 @@ func TestValidate_PeerNotRecognized_FirstApplicationReturnedError(t *testing.T)
 			application1,
 			application2},
 		allowList:           EmptyAllowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 
 	err = policy.Validate(peerOperatorPublicKey)
 	testutils.AssertAnyErrorInChainMatchesTarget(t, applicationError, err)
 }
 
-func TestValidate_PeerRecognized_Cached(t *testing.T) {
+func TestValidate_PeerRecognized_Rechecked(t *testing.T) {
 	_, peerOperatorPublicKey, err := operator.GenerateKeyPair(
 		local_v1.DefaultCurve,
 	)
@@ -165,7 +160,6 @@ func TestValidate_PeerRecognized_Cached(t *testing.T) {
 	policy := &anyApplicationPolicy{
 		applications:        []Application{application},
 		allowList:           EmptyAllowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 
@@ -175,16 +169,15 @@ func TestValidate_PeerRecognized_Cached(t *testing.T) {
 	}
 
 	// Ensure the application does not recognize the operator anymore.
-	// Validation should still succeed, since the cached result should be used.
+	// Validation should fail because positive results are rechecked to avoid
+	// accepting peers whose application recognition was revoked.
 	application.setIsRecognized(peerOperatorPublicKey, result{
 		isRecognized: false,
 		err:          nil,
 	})
 
 	err = policy.Validate(peerOperatorPublicKey)
-	if err != nil {
-		t.Fatal(err)
-	}
+	testutils.AssertErrorsSame(t, errNotRecognized, err)
 }
 
 func TestValidate_PeerNotRecognized_CacheEmptied(t *testing.T) {
@@ -204,7 +197,6 @@ func TestValidate_PeerNotRecognized_CacheEmptied(t *testing.T) {
 	policy := &anyApplicationPolicy{
 		applications:        []Application{application},
 		allowList:           EmptyAllowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 
@@ -239,7 +231,6 @@ func TestValidate_PeerNotRecognized_Cached(t *testing.T) {
 	policy := &anyApplicationPolicy{
 		applications:        []Application{application},
 		allowList:           EmptyAllowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 
@@ -274,7 +265,6 @@ func TestValidate_PeerRecognized_CacheEmptied(t *testing.T) {
 	policy := &anyApplicationPolicy{
 		applications:        []Application{application},
 		allowList:           EmptyAllowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 
@@ -312,7 +302,6 @@ func TestValidate_PeerIsAllowlistedNode(t *testing.T) {
 	policy := &anyApplicationPolicy{
 		applications:        []Application{newMockApplication()},
 		allowList:           allowList,
-		positiveResultCache: cache.NewTimeCache(cachingPeriod),
 		negativeResultCache: cache.NewTimeCache(cachingPeriod),
 	}
 

pkg/maintainer/spv/spv.go

@@ -272,6 +272,15 @@ func isInputCurrentWalletsMainUTXO(
 	if err != nil {
 		return false, fmt.Errorf("failed to get previous transaction: [%v]", err)
 	}
+	if fundingOutputIndex >= uint32(len(previousTransaction.Outputs)) {
+		return false, fmt.Errorf(
+			"funding output index [%d] out of range for transaction [%s] "+
+				"with [%d] outputs",
+			fundingOutputIndex,
+			fundingTxHash.String(),
+			len(previousTransaction.Outputs),
+		)
+	}
 	fundingOutputValue := previousTransaction.Outputs[fundingOutputIndex].Value
 
 	// Assume the input is the main UTXO and calculate hash.

pkg/maintainer/spv/spv_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/hex"
 	"math/big"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/keep-network/keep-core/internal/testutils"
@@ -217,6 +218,55 @@ func TestUniqueWalletPublicKeyHashes(t *testing.T) {
 	}
 }
 
+func TestIsInputCurrentWalletsMainUTXO_OutOfRangeFundingOutput(t *testing.T) {
+	bytesFromHex := func(str string) []byte {
+		value, err := hex.DecodeString(str)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		return value
+	}
+
+	txFromHex := func(str string) *bitcoin.Transaction {
+		transaction := new(bitcoin.Transaction)
+		err := transaction.Deserialize(bytesFromHex(str))
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		return transaction
+	}
+
+	btcChain := newLocalBitcoinChain()
+	fundingTransaction := txFromHex(
+		"0100000000010110a15e879b7e8b07df62772579a64bf2b409409bbcc8bc2c7f6e39" +
+			"31dc615e920100000000ffffffff02042900000000000017a9143ec459d0f3c29286" +
+			"ae5df5fcc421e2786024277e87b4121600000000001600148db50eb52063ea9d98b3" +
+			"eac91489a90f738986f6024830450221009740ad12d2e74c00ccb4741d533d2ecd69" +
+			"02289144c4626508afb61eed790c97022006e67179e8e2a63dc4f1ab758867d8bbfe" +
+			"0a2b67682be6dadfa8e07d3b7ba04d012103989d253b17a6a0f41838b84ff0d20e88" +
+			"98f9d7b1a98f2564da4cc29dcf8581d900000000",
+	)
+	if err := btcChain.BroadcastTransaction(fundingTransaction); err != nil {
+		t.Fatal(err)
+	}
+
+	_, err := isInputCurrentWalletsMainUTXO(
+		fundingTransaction.Hash(),
+		2,
+		[20]byte{},
+		btcChain,
+		newLocalChain(),
+	)
+	if err == nil {
+		t.Fatal("expected out-of-range funding output error")
+	}
+	if !strings.Contains(err.Error(), "funding output index [2] out of range") {
+		t.Fatalf("unexpected error: [%v]", err)
+	}
+}
+
 func TestIsInputCurrentWalletsMainUTXO(t *testing.T) {
 	bytesFromHex := func(str string) []byte {
 		value, err := hex.DecodeString(str)