Merge pull request #1358 from bitromortac/2607-fix-32-overflow

firewall: fix 32-bit int truncation and optimize CryptoRandIntn

docs/release-notes/release-notes-0.17.1.md

@@ -54,6 +54,14 @@
   -V`](https://github.com/lightninglabs/lightning-terminal/pull/1337): The `-V`
   flag now prints litd's version instead of the integrated lnd version.
 
+* [Refactor privacy mapper to prevent 32-bit truncation and optimize
+  allocations](https://github.com/lightninglabs/lightning-terminal/pull/1358):
+  Refactored the privacy mapper's random number generation to use `int64`
+  instead of `int` to prevent architecture-dependent truncation on 32-bit
+  runtimes. This represents a breaking change to the `NewPrivacyMapper` and
+  `CryptoRandIntn` functions. The PR also introduced a `sync.Pool` for
+  `*big.Int` to optimize allocations.
+
 ## RPC Updates
 
 ## Integrated Binary Updates

firewall/privacy_mapper.go

@@ -7,9 +7,11 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"math/big"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
@@ -61,14 +63,14 @@ var _ mid.RequestInterceptor = (*PrivacyMapper)(nil)
 // requests to their real values and vice versa for responses.
 type PrivacyMapper struct {
 	db        firewalldb.PrivacyMapper
-	randIntn  func(int) (int, error)
+	randIntn  func(int64) (int64, error)
 	sessionDB firewalldb.SessionDB
 }
 
 // NewPrivacyMapper returns a new instance of PrivacyMapper. The randIntn
 // function is used to draw randomness for request field obfuscation.
 func NewPrivacyMapper(newDB firewalldb.PrivacyMapper,
-	randIntn func(int) (int, error),
+	randIntn func(int64) (int64, error),
 	sessionDB firewalldb.SessionDB) *PrivacyMapper {
 
 	return &PrivacyMapper{
@@ -379,7 +381,7 @@ func handleGetInfoResponse(db firewalldb.PrivacyMapDB,
 
 func handleFwdHistoryResponse(db firewalldb.PrivacyMapDB,
 	flags session.PrivacyFlags,
-	randIntn func(int) (int, error)) func(ctx context.Context,
+	randIntn func(int64) (int64, error)) func(ctx context.Context,
 	r *lnrpc.ForwardingHistoryResponse) (proto.Message, error) {
 
 	return func(ctx context.Context, r *lnrpc.ForwardingHistoryResponse) (
@@ -582,7 +584,7 @@ func handleListChannelsRequest(db firewalldb.PrivacyMapDB,
 
 func handleListChannelsResponse(db firewalldb.PrivacyMapDB,
 	flags session.PrivacyFlags,
-	randIntn func(int) (int, error)) func(ctx context.Context,
+	randIntn func(int64) (int64, error)) func(ctx context.Context,
 	r *lnrpc.ListChannelsResponse) (proto.Message, error) {
 
 	return func(ctx context.Context, r *lnrpc.ListChannelsResponse) (
@@ -866,7 +868,7 @@ func handleUpdatePolicyResponse(db firewalldb.PrivacyMapDB,
 
 func handleWalletBalanceResponse(_ firewalldb.PrivacyMapDB,
 	flags session.PrivacyFlags,
-	randIntn func(int) (int, error)) func(ctx context.Context,
+	randIntn func(int64) (int64, error)) func(ctx context.Context,
 	r *lnrpc.WalletBalanceResponse) (proto.Message, error) {
 
 	return func(_ context.Context, r *lnrpc.WalletBalanceResponse) (
@@ -945,7 +947,7 @@ func handleWalletBalanceResponse(_ firewalldb.PrivacyMapDB,
 
 func handleClosedChannelsResponse(db firewalldb.PrivacyMapDB,
 	flags session.PrivacyFlags,
-	randIntn func(int) (int, error)) func(ctx context.Context,
+	randIntn func(int64) (int64, error)) func(ctx context.Context,
 	r *lnrpc.ClosedChannelsResponse) (proto.Message, error) {
 
 	return func(ctx context.Context, r *lnrpc.ClosedChannelsResponse) (
@@ -1065,7 +1067,7 @@ func handleClosedChannelsResponse(db firewalldb.PrivacyMapDB,
 // channel.
 func obfuscatePendingChannel(ctx context.Context,
 	c *lnrpc.PendingChannelsResponse_PendingChannel,
-	tx firewalldb.PrivacyMapTx, randIntn func(int) (int, error),
+	tx firewalldb.PrivacyMapTx, randIntn func(int64) (int64, error),
 	flags session.PrivacyFlags) (
 	*lnrpc.PendingChannelsResponse_PendingChannel, error) {
 
@@ -1142,7 +1144,7 @@ func obfuscatePendingChannel(ctx context.Context,
 
 func handlePendingChannelsResponse(db firewalldb.PrivacyMapDB,
 	flags session.PrivacyFlags,
-	randIntn func(int) (int, error)) func(ctx context.Context,
+	randIntn func(int64) (int64, error)) func(ctx context.Context,
 	r *lnrpc.PendingChannelsResponse) (proto.Message, error) {
 
 	return func(ctx context.Context, r *lnrpc.PendingChannelsResponse) (
@@ -1729,7 +1731,7 @@ func handleConnectPeerRequest(db firewalldb.PrivacyMapDB,
 }
 
 // maybeHideAmount hides an amount if the privacy flag is not set.
-func maybeHideAmount(flags session.PrivacyFlags, randIntn func(int) (int,
+func maybeHideAmount(flags session.PrivacyFlags, randIntn func(int64) (int64,
 	error), a int64) (int64, error) {
 
 	if !flags.Contains(session.ClearAmounts) {
@@ -1748,8 +1750,8 @@ func maybeHideAmount(flags session.PrivacyFlags, randIntn func(int) (int,
 
 // hideAmount symmetrically randomizes an amount around a given relative
 // variation interval. relativeVariation should be between 0 and 1.
-func hideAmount(randIntn func(n int) (int, error), relativeVariation float64,
-	amount uint64) (uint64, error) {
+func hideAmount(randIntn func(n int64) (int64, error),
+	relativeVariation float64, amount uint64) (uint64, error) {
 
 	if relativeVariation < 0 || relativeVariation > 1 {
 		return 0, fmt.Errorf("hide amount: relative variation is not "+
@@ -1757,6 +1759,11 @@ func hideAmount(randIntn func(n int) (int, error), relativeVariation float64,
 			relativeVariation)
 	}
 
+	if amount > math.MaxInt64 {
+		return 0, fmt.Errorf("hide amount: amount %d overflows "+
+			"int64", amount)
+	}
+
 	if amount == 0 {
 		return 0, nil
 	}
@@ -1765,8 +1772,13 @@ func hideAmount(randIntn func(n int) (int, error), relativeVariation float64,
 	// between 0 and 1.
 	fuzzInterval := uint64(float64(amount) * relativeVariation)
 
-	amountMin := int(amount - fuzzInterval)
-	amountMax := int(amount + fuzzInterval)
+	if amount+fuzzInterval > math.MaxInt64 {
+		return 0, fmt.Errorf("hide amount: amount %d with variation "+
+			"overflows int64", amount)
+	}
+
+	amountMin := int64(amount - fuzzInterval)
+	amountMax := int64(amount + fuzzInterval)
 
 	randAmount, err := randBetween(randIntn, amountMin, amountMax)
 	if err != nil {
@@ -1778,7 +1790,7 @@ func hideAmount(randIntn func(n int) (int, error), relativeVariation float64,
 
 // hideTimestamp symmetrically randomizes a unix timestamp given an absolute
 // variation interval. The random input is expected to be rand.Intn.
-func hideTimestamp(randIntn func(n int) (int, error),
+func hideTimestamp(randIntn func(n int64) (int64, error),
 	absoluteVariation time.Duration,
 	timestamp time.Time) (time.Time, error) {
 
@@ -1802,18 +1814,20 @@ func hideTimestamp(randIntn func(n int) (int, error),
 	timeMax := timestamp.Add(absoluteVariation)
 
 	timeNs, err := randBetween(
-		randIntn, int(timeMin.UnixNano()), int(timeMax.UnixNano()),
+		randIntn, timeMin.UnixNano(), timeMax.UnixNano(),
 	)
 	if err != nil {
 		return time.Time{}, err
 	}
 
-	return time.Unix(0, int64(timeNs)), nil
+	return time.Unix(0, timeNs), nil
 }
 
 // randBetween generates a random number between [min, max) given a source of
 // randomness.
-func randBetween(randIntn func(int) (int, error), min, max int) (int, error) {
+func randBetween(randIntn func(int64) (int64, error),
+	min, max int64) (int64, error) {
+
 	if max < min {
 		return 0, fmt.Errorf("min is not allowed to be greater than "+
 			"max, (min: %v, max: %v)", min, max)
@@ -1833,7 +1847,7 @@ func randBetween(randIntn func(int) (int, error), min, max int) (int, error) {
 }
 
 // hideBool generates a random bool given a random input.
-func hideBool(randIntn func(n int) (int, error)) (bool, error) {
+func hideBool(randIntn func(n int64) (int64, error)) (bool, error) {
 	random, err := randIntn(2)
 	if err != nil {
 		return false, err
@@ -1844,18 +1858,31 @@ func hideBool(randIntn func(n int) (int, error)) (bool, error) {
 	return random >= 1, nil
 }
 
+// bigIntPool is a pool of *big.Int values to avoid frequent heap allocations in
+// CryptoRandIntn.
+var bigIntPool = sync.Pool{
+	New: func() any {
+		return new(big.Int)
+	},
+}
+
 // CryptoRandIntn generates a random number between [0, n).
-func CryptoRandIntn(n int) (int, error) {
+func CryptoRandIntn(n int64) (int64, error) {
 	if n == 0 {
 		return 0, nil
 	}
 
-	nBig, err := rand.Int(rand.Reader, big.NewInt(int64(n)))
+	nBig := bigIntPool.Get().(*big.Int)
+	defer bigIntPool.Put(nBig)
+
+	nBig.SetInt64(n)
+
+	randBig, err := rand.Int(rand.Reader, nBig)
 	if err != nil {
 		return 0, err
 	}
 
-	return int(nBig.Int64()), nil
+	return randBig.Int64(), nil
 }
 
 // ObfuscateConfig alters the config string by replacing sensitive data with

firewall/privacy_mapper_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"math"
 	"testing"
 	"time"
 
@@ -919,7 +920,9 @@ func TestPrivacyMapper(t *testing.T) {
 			require.NoError(t, err)
 
 			// randIntn is used for deterministic testing.
-			randIntn := func(n int) (int, error) { return 100, nil }
+			randIntn := func(n int64) (int64, error) {
+				return 100, nil
+			}
 			p := NewPrivacyMapper(db, randIntn, pd)
 
 			rawMsg, err := proto.Marshal(test.msg)
@@ -1188,8 +1191,10 @@ var _ firewalldb.PrivacyMapDB = (*mockPrivacyMapDB)(nil)
 
 // TestRandBetween tests random number generation for numbers in an interval.
 func TestRandBetween(t *testing.T) {
-	min := 0
-	max := 10
+	var (
+		min int64
+		max int64 = 10
+	)
 
 	for i := 0; i < 100; i++ {
 		val, err := randBetween(CryptoRandIntn, min, max)
@@ -1210,38 +1215,40 @@ func TestHideAmount(t *testing.T) {
 	tests := []struct {
 		name      string
 		amount    uint64
-		randIntFn func(int) (int, error)
+		randIntFn func(int64) (int64, error)
 		expected  uint64
 	}{
 		{
 			name:      "zero test amount",
-			randIntFn: func(int) (int, error) { return 0, nil },
+			randIntFn: func(int64) (int64, error) { return 0, nil },
 		},
 		{
 			name:      "test small amount",
-			randIntFn: func(int) (int, error) { return 0, nil },
+			randIntFn: func(int64) (int64, error) { return 0, nil },
 			amount:    1,
 			expected:  1,
 		},
 		{
 			name:      "min value",
-			randIntFn: func(int) (int, error) { return 0, nil },
+			randIntFn: func(int64) (int64, error) { return 0, nil },
 			amount:    testAmount,
 			expected:  lowerBound,
 		},
 		{
 			name: "max value",
-			randIntFn: func(int) (int, error) {
-				return int(upperBound - lowerBound), nil
+			randIntFn: func(n int64) (int64, error) {
+				return int64(upperBound - lowerBound), nil
 			},
 			amount:   testAmount,
 			expected: upperBound,
 		},
 		{
-			name:      "some fuzz",
-			randIntFn: func(int) (int, error) { return 123, nil },
-			amount:    testAmount,
-			expected:  lowerBound + 123,
+			name: "some fuzz",
+			randIntFn: func(int64) (int64, error) {
+				return 123, nil
+			},
+			amount:   testAmount,
+			expected: lowerBound + 123,
 		},
 	}
 
@@ -1270,6 +1277,33 @@ func TestHideAmount(t *testing.T) {
 			require.NoError(t, err)
 		}
 	})
+
+	t.Run("hideAmount overflow validation", func(t *testing.T) {
+		// amount > math.MaxInt64 should fail.
+		_, err := hideAmount(
+			CryptoRandIntn,
+			relativeVariation,
+			math.MaxInt64+1,
+		)
+		require.Error(t, err)
+
+		// amount <= math.MaxInt64 but amount + fuzzInterval >
+		// math.MaxInt64 should fail.
+		_, err = hideAmount(
+			CryptoRandIntn,
+			0.05,
+			math.MaxInt64-100,
+		)
+		require.Error(t, err)
+
+		// A value that just fits should succeed.
+		_, err = hideAmount(
+			CryptoRandIntn,
+			0.1,
+			922337203685477580,
+		)
+		require.NoError(t, err)
+	})
 }
 
 // TestHideTimestamp test correct timestamp hiding.
@@ -1281,31 +1315,33 @@ func TestHideTimestamp(t *testing.T) {
 
 	tests := []struct {
 		name      string
-		randIntFn func(int) (int, error)
+		randIntFn func(int64) (int64, error)
 		timestamp time.Time
 		expected  time.Time
 	}{
 		{
 			name:      "zero timestamp",
-			randIntFn: func(int) (int, error) { return 0, nil },
+			randIntFn: func(int64) (int64, error) { return 0, nil },
 		},
 		{
 			name:      "min value",
-			randIntFn: func(int) (int, error) { return 0, nil },
+			randIntFn: func(int64) (int64, error) { return 0, nil },
 			timestamp: timestamp,
 			expected:  lowerBound,
 		},
 		{
 			name: "max value",
-			randIntFn: func(int) (int, error) {
-				return int(upperBound.Sub(lowerBound)), nil
+			randIntFn: func(n int64) (int64, error) {
+				return int64(upperBound.Sub(lowerBound)), nil
 			},
 			timestamp: timestamp,
 			expected:  upperBound,
 		},
 		{
-			name:      "some fuzz",
-			randIntFn: func(int) (int, error) { return 123, nil },
+			name: "some fuzz",
+			randIntFn: func(int64) (int64, error) {
+				return 123, nil
+			},
 			timestamp: timestamp,
 			expected:  lowerBound.Add(time.Duration(123)),
 		},
@@ -1328,17 +1364,37 @@ func TestHideTimestamp(t *testing.T) {
 
 // TestHideBool test correct boolean hiding.
 func TestHideBool(t *testing.T) {
-	val, err := hideBool(func(int) (int, error) { return 100, nil })
-	require.NoError(t, err)
-	require.True(t, val)
+	tests := []struct {
+		name     string
+		random   int64
+		expected bool
+	}{
+		{
+			name:     "random value",
+			random:   100,
+			expected: true,
+		},
+		{
+			name:     "exact threshold value",
+			random:   1,
+			expected: true,
+		},
+		{
+			name:     "below threshold value",
+			random:   0,
+			expected: false,
+		},
+	}
 
-	val, err = hideBool(func(int) (int, error) { return 1, nil })
-	require.NoError(t, err)
-	require.True(t, val)
-
-	val, err = hideBool(func(int) (int, error) { return 0, nil })
-	require.NoError(t, err)
-	require.False(t, val)
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			val, err := hideBool(func(n int64) (int64, error) {
+				return tc.random, nil
+			})
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, val)
+		})
+	}
 }
 
 // TestObfuscateConfig tests that we substitute substrings in the config
@@ -1605,3 +1661,27 @@ func variance(numbers []uint64) uint64 {
 
 	return uint64(sum)
 }
+
+// Test32BitOverflowAndTruncation ensures values > MaxInt32 do not truncate on
+// 32-bit runtimes.
+func Test32BitOverflowAndTruncation(t *testing.T) {
+	largeValue := int64(math.MaxInt32) + 1
+
+	// Ensure randBetween successfully operates on values larger than
+	// MaxInt32 without any architecture-dependent truncation.
+	mockRand := func(n int64) (int64, error) {
+		return n - 1, nil
+	}
+
+	val, err := randBetween(mockRand, largeValue, largeValue+10)
+	require.NoError(t, err)
+	require.Equal(t, largeValue+9, val)
+
+	// Ensure hideTimestamp works with a 64-bit nanosecond timestamp
+	// representing a real far-future date (> MaxInt32).
+	farFutureTime := time.Unix(0, 1600000000000000000) // ~Year 2020 in ns
+	variation := 10 * time.Minute
+	resTime, err := hideTimestamp(mockRand, variation, farFutureTime)
+	require.NoError(t, err)
+	require.NotEmpty(t, resTime)
+}