lnwallet: prevent transaction pagination overflow
What changed, and why it matters
This commit fixes a pagination bug in LND's wallet transaction listing. Previously, adding a user-controlled 'offset' and 'limit' together as 32-bit unsigned integers could wrap around to a tiny number, causing the code to slice transaction data using an unexpectedly small 'end' index and potentially return wrong results or behave inconsistently. The fix performs pagination using 64-bit arithmetic and compares the limit against the remaining items before deciding where to stop, preventing the overflow.
Review callers of ListTransactionDetails to ensure offset/limit are not attacker-controllable without authentication, and confirm the new helper is used consistently elsewhere if pagination logic is duplicated. No immediate emergency action is indicated beyond applying the patch.
Security signals we found
Integer overflow in pagination bounds calculation (uint32 addition)
Caller-controlled offset and limit values
Potential incorrect slicing of transaction detail slice
New unit tests explicitly exercise uint32 overflow boundary
Evidence from the diff
ListTransactionDetails in lnwallet/btcwallet/btcwallet.go previously computed end = indexOffset + maxTransactions using uint32. If indexOffset was large and maxTransactions was also large, the sum could overflow and end could become smaller than indexOffset. The new helper transactionDetailsPage converts values to uint64, checks if indexOffset exceeds total length, computes remaining = total - first, and only sets end = first + limit when limit < remaining. This avoids adding offset and limit before clamping and prevents wraparound. Unit tests cover cases including math.MaxUint32 limit/offset.
Changed components
lnwallet/btcwallet/btcwallet.golnwallet/btcwallet/btcwallet_test.goBtcWallet.ListTransactionDetailstransactionDetailsPage helperInspect captured patch +110 / −25
### lnwallet/btcwallet/btcwallet.go
@@ -1554,6 +1554,38 @@ func unminedTransactionsToDetail(
return txDetail, nil
}
+// transactionDetailsPage applies the requested offset and limit to a set of
+// transaction details. A zero limit means that all remaining transactions are
+// returned.
+func transactionDetailsPage(txDetails []*lnwallet.TransactionDetail,
+ indexOffset, maxTransactions uint32) ([]*lnwallet.TransactionDetail,
+ uint64, uint64) {
+
+ total := uint64(len(txDetails))
+ first := uint64(indexOffset)
+ if first >= total {
+ return []*lnwallet.TransactionDetail{}, 0, 0
+ }
+
+ end := total
+ if maxTransactions != 0 {
+ // Compare the limit to the remaining count. This avoids adding
+ // caller-controlled values before deciding whether to clamp the
+ // requested end.
+ limit := uint64(maxTransactions)
+ remaining := total - first
+ if limit < remaining {
+ end = first + limit
+ }
+ }
+
+ // Both bounds are no greater than len(txDetails), so these conversions
+ // are safe on both 32-bit and 64-bit platforms.
+ page := txDetails[int(first):int(end)]
+
+ return page, first, end - 1
+}
+
// ListTransactionDetails returns a list of all transactions which are relevant
// to the wallet over [startHeight;endHeight]. If start height is greater than
// end height, the transactions will be retrieved in reverse order. To include
@@ -1607,32 +1639,11 @@ func (b *BtcWallet) ListTransactionDetails(startHeight, endHeight int32,
txDetails = append(txDetails, detail)
}
- // Return empty transaction list, if offset is more than all
- // transactions.
- if int(indexOffset) >= len(txDetails) {
- txDetails = []*lnwallet.TransactionDetail{}
-
- return txDetails, 0, 0, nil
- }
-
- end := indexOffset + maxTransactions
-
- // If maxTransactions is set to 0, then we'll return all transactions
- // starting from the offset.
- if maxTransactions == 0 {
- end = uint32(len(txDetails))
- txDetails = txDetails[indexOffset:end]
-
- return txDetails, uint64(indexOffset), uint64(end - 1), nil
- }
-
- if end > uint32(len(txDetails)) {
- end = uint32(len(txDetails))
- }
-
- txDetails = txDetails[indexOffset:end]
+ page, firstIndex, lastIndex := transactionDetailsPage(
+ txDetails, indexOffset, maxTransactions,
+ )
- return txDetails, uint64(indexOffset), uint64(end - 1), nil
+ return page, firstIndex, lastIndex, nil
}
// txSubscriptionClient encapsulates the transaction notification client from
### lnwallet/btcwallet/btcwallet_test.go
@@ -1,6 +1,7 @@
package btcwallet
import (
+ "math"
"testing"
"github.com/btcsuite/btcd/btcjson"
@@ -138,6 +139,79 @@ func TestPreviousOutpoints(t *testing.T) {
}
}
+// TestTransactionDetailsPage verifies the bounds returned for transaction
+// pagination, including requests that would overflow with uint32 addition.
+func TestTransactionDetailsPage(t *testing.T) {
+ t.Parallel()
+
+ txDetails := []*lnwallet.TransactionDetail{{}, {}, {}, {}}
+
+ testCases := []struct {
+ name string
+ offset uint32
+ limit uint32
+ expectedPage []*lnwallet.TransactionDetail
+ expectedFirst uint64
+ expectedLast uint64
+ }{
+ {
+ name: "zero limit returns remainder",
+ offset: 1,
+ expectedPage: txDetails[1:],
+ expectedFirst: 1,
+ expectedLast: 3,
+ },
+ {
+ name: "limit selects page",
+ offset: 1,
+ limit: 2,
+ expectedPage: txDetails[1:3],
+ expectedFirst: 1,
+ expectedLast: 2,
+ },
+ {
+ name: "limit exceeds remainder",
+ offset: 2,
+ limit: 10,
+ expectedPage: txDetails[2:],
+ expectedFirst: 2,
+ expectedLast: 3,
+ },
+ {
+ name: "offset plus limit exceeds uint32",
+ offset: 1,
+ limit: math.MaxUint32,
+ expectedPage: txDetails[1:],
+ expectedFirst: 1,
+ expectedLast: 3,
+ },
+ {
+ name: "offset equals transaction count",
+ offset: uint32(len(txDetails)),
+ limit: 1,
+ expectedPage: []*lnwallet.TransactionDetail{},
+ },
+ {
+ name: "maximum offset",
+ offset: math.MaxUint32,
+ limit: 1,
+ expectedPage: []*lnwallet.TransactionDetail{},
+ },
+ }
+
+ for _, testCase := range testCases {
+ t.Run(testCase.name, func(t *testing.T) {
+ page, first, last := transactionDetailsPage(
+ txDetails, testCase.offset, testCase.limit,
+ )
+
+ require.Equal(t, testCase.expectedPage, page)
+ require.Equal(t, testCase.expectedFirst, first)
+ require.Equal(t, testCase.expectedLast, last)
+ })
+ }
+}
+
// TestCheckMempoolAcceptance asserts the CheckMempoolAcceptance behaves as
// expected.
func TestCheckMempoolAcceptance(t *testing.T) {Why this scored 40/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.