diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f17508e..2d4a4cc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,20 @@ The most recent changes are listed first. ## [Unreleased] +### Changed + +- `GetAddressUtxos` and `GetAddressUtxosStream` now pass `startHeight` and + `maxEntries` to the backend `getaddressutxos` RPC, instead of only applying + them to the reply. No backend implements the arguments yet, and a backend + that doesn't implement them ignores the extra JSON keys, so on its own this + changes nothing: the client-side filter stays, and correctness never depends + on the backend honoring them. It is the lightwalletd half of moving the + limits to where the work is done, which is what the unremediated part of + GHSA-x4m7-3gpp-xc36 needs -- today a request naming a single address with a + large UTXO set makes the node produce that entire set, however narrow a + height range or however few entries the client asked for. A request that + sets neither argument is serialized exactly as it was before. + ### Fixed - `GetTaddressBalance` now rejects an address list longer than the same 10,000 diff --git a/common/common.go b/common/common.go index 510ef140..1a1d1e07 100644 --- a/common/common.go +++ b/common/common.go @@ -172,6 +172,9 @@ type ( // zcashd rpc "getaddressutxos" ZcashdRpcRequestGetaddressutxos struct { Addresses []string `json:"addresses"` + // Optional; a backend that doesn't implement these ignores them. + StartHeight uint64 `json:"startHeight,omitempty"` + MaxEntries uint32 `json:"maxEntries,omitempty"` } ZcashdRpcReplyGetaddressutxos struct { Address string diff --git a/frontend/frontend_test.go b/frontend/frontend_test.go index eee70419..2c5a0462 100644 --- a/frontend/frontend_test.go +++ b/frontend/frontend_test.go @@ -625,6 +625,73 @@ type testgettx struct { walletrpc.CompactTxStreamer_GetTaddressTransactionsServer } +func TestGetAddressUtxosPushesDownLimits(t *testing.T) { + testT = t + defer resetGlobals() + lwd, _ := testsetup() + + taddr := "t1" + strings.Repeat("a", 33) + utxo := func(height int) common.ZcashdRpcReplyGetaddressutxos { + return common.ZcashdRpcReplyGetaddressutxos{ + Address: taddr, + Txid: "0788e4dc9973cd9a54e0f4d51ec96f4b8e6a8e0f8a1e1e9e4b2c2a1d0e0f0a0b", + OutputIndex: 0, + Script: "76a914000000000000000000000000000000000000000088ac", + Satoshis: 1000, + Height: height, + } + } + + // Model a backend that doesn't implement the new arguments: it returns + // every utxo, in chain order, whatever the request asked for. + var sent json.RawMessage + common.RawRequest = func(ctx context.Context, method string, params []json.RawMessage) (json.RawMessage, error) { + if method != "getaddressutxos" { + testT.Fatal("unexpected method", method) + } + sent = params[0] + return json.Marshal([]common.ZcashdRpcReplyGetaddressutxos{ + utxo(100), utxo(200), utxo(300), utxo(400), + }) + } + + reply, err := lwd.GetAddressUtxos(context.Background(), &walletrpc.GetAddressUtxosArg{ + Addresses: []string{taddr}, + StartHeight: 200, + MaxEntries: 2, + }) + if err != nil { + t.Fatal("GetAddressUtxos failed:", err) + } + var req common.ZcashdRpcRequestGetaddressutxos + if err := json.Unmarshal(sent, &req); err != nil { + t.Fatal("could not unmarshal getaddressutxos request") + } + if req.StartHeight != 200 || req.MaxEntries != 2 { + t.Fatal("expected the limits to reach the backend, got:", string(sent)) + } + // The backend ignored them, so the client-side filter must still hold. + heights := make([]uint64, 0) + for _, u := range reply.AddressUtxos { + heights = append(heights, u.Height) + } + if !reflect.DeepEqual(heights, []uint64{200, 300}) { + t.Fatal("expected utxos at heights 200 and 300, got:", heights) + } + + // A request that sets neither limit is byte-for-byte what it was before + // the arguments existed, so an unpatched backend sees no change. + _, err = lwd.GetAddressUtxos(context.Background(), &walletrpc.GetAddressUtxosArg{ + Addresses: []string{taddr}, + }) + if err != nil { + t.Fatal("GetAddressUtxos failed:", err) + } + if want := `{"addresses":["` + taddr + `"]}`; string(sent) != want { + t.Fatal("expected unset limits to be omitted, got:", string(sent)) + } +} + func (tg *testgettx) Context() context.Context { return context.Background() } diff --git a/frontend/service.go b/frontend/service.go index 68f0b3f5..2da9d6e0 100644 --- a/frontend/service.go +++ b/frontend/service.go @@ -918,7 +918,9 @@ func getAddressUtxos(ctx context.Context, arg *walletrpc.GetAddressUtxosArg, f f addresses = append(addresses, a) } addrList := &common.ZcashdRpcRequestGetaddressutxos{ - Addresses: addresses, + Addresses: addresses, + StartHeight: arg.StartHeight, + MaxEntries: arg.MaxEntries, } param, err := json.Marshal(addrList) if err != nil { @@ -946,6 +948,7 @@ func getAddressUtxos(ctx context.Context, arg *walletrpc.GetAddressUtxosArg, f f } n := 0 for _, utxo := range utxosReply { + // Re-apply the limits; a backend that ignored them sent everything. if uint64(utxo.Height) < arg.StartHeight { continue } @@ -953,6 +956,7 @@ func getAddressUtxos(ctx context.Context, arg *walletrpc.GetAddressUtxosArg, f f if arg.MaxEntries > 0 && uint32(n) > arg.MaxEntries { break } + txidBigEndian, err := hex.DecodeString(utxo.Txid) if err != nil { return status.Errorf(codes.Internal,