Skip to content

Commit 3f3f610

Browse files
committed
fix(multisig,boolpolicy): verify spend tx matches approved SpendRequest
EndorseSpendView in both token/services/ttx/multisig/spend.go and token/services/ttx/boolpolicy/spend.go received a transaction from the initiator and signed it without checking that the transaction actually consumed the token referenced by the SpendRequest the co-owner approved. The multisig variant carried an explicit 'TODO: check tx matches request' marker; the policy variant had the same gap without a marker. A malicious or buggy initiator could ask a co-owner to approve a SpendRequest for one token and then deliver a transaction that consumes a different token co-owned by the same group. The co-owner would sign the substituted transaction with no diagnostic, breaking the consent property that multisig and AND-policy spends rely on. This change adds a verifySpendTxMatchesRequest helper in each package. After ttx.ReceiveTransaction returns, the helper extracts the input ids from the transaction's audit record and rejects the flow before EndorseView is invoked unless every input id equals request.Token.Id. The comparison rule is split into a pure helper verifyInputIDsMatchExpected so it can be unit-tested without constructing a full ttx.Transaction. Documentation in docs/services/ttx.md gains a new 'Spend Coordination Wire Flow' section with a Mermaid diagram describing the request, delivery, and verification phases shared by the multisig and policy variants. Closes #1682 Signed-off-by: SuyashAlphaC <suyashagrawal862@gmail.com>
1 parent d370298 commit 3f3f610

5 files changed

Lines changed: 242 additions & 1 deletion

File tree

docs/services/ttx.md

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,43 @@ _, err = context.RunView(ttx.NewCollectEndorsementsView(tx))
208208

209209
Co-owners run `EndorseSpendView` (via `EndorseSpend`) on their side, which ACKs the spend request and then endorses the assembled transaction.
210210

211+
#### Spend Coordination Wire Flow
212+
213+
The same coordination protocol is implemented in `token/services/ttx/multisig/spend.go` (for AND multisig) and in `token/services/ttx/boolpolicy/spend.go` (for AND policies). Both follow the shape below; the responder must verify that the assembled transaction actually consumes the token referenced by the `SpendRequest` it approved.
214+
215+
```mermaid
216+
sequenceDiagram
217+
autonumber
218+
participant I as Initiator (RequestSpendView)
219+
participant R as Co-owner (EndorseSpendView)
220+
221+
rect rgba(230, 230, 250, 0.35)
222+
Note over I,R: Phase 1 - Spend approval request
223+
I->>R: SpendRequest{Token: UnspentToken to spend}
224+
R->>R: ReceiveSpendRequest, decide to approve
225+
R-->>I: SpendResponse{}
226+
end
227+
228+
rect rgba(255, 245, 238, 0.5)
229+
Note over I,R: Phase 2 - Transaction assembly and delivery
230+
I->>I: Assemble transaction consuming SpendRequest.Token
231+
I->>R: Transaction (via ttx.ReceiveTransaction)
232+
end
233+
234+
rect rgba(240, 255, 240, 0.45)
235+
Note over R: Phase 3 - Verification (required before signing)
236+
R->>R: Extract input IDs from tx.Request().AuditRecord
237+
alt every input id == SpendRequest.Token.Id
238+
R->>R: Run EndorseView(tx) and sign
239+
R-->>I: Signed transaction
240+
else mismatch
241+
R-->>I: Reject with error; no signature is produced
242+
end
243+
end
244+
```
245+
246+
The Phase 3 check is what links the artifact a co-owner approves (the `SpendRequest`) to the artifact they sign (the assembled `tx`). Without it, a co-owner who reviews and approves a spend for token `T_a` could be made to sign a transaction consuming a different token `T_b` co-owned by the same group.
247+
211248
#### Wallet and Authorization
212249

213250
The `boolpolicy.OwnerWallet` (in `token/services/ttx/boolpolicy/wallet.go`) wraps a standard owner wallet and filters the token list to policy-type tokens. `VerifyApprover` can be used to assert that a given identity is one of the named component identities before allowing a spend.

token/services/ttx/boolpolicy/spend.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ SPDX-License-Identifier: Apache-2.0
1111
package boolpolicy
1212

1313
import (
14+
"context"
1415
"slices"
1516
"time"
1617

@@ -223,9 +224,48 @@ func (a *EndorseSpendView) Call(context view.Context) (interface{}, error) {
223224
if err != nil {
224225
return nil, errors.Wrap(err, "failed to receive transaction")
225226
}
227+
// Reject any received tx that consumes a different token from the one this
228+
// co-owner approved in SpendRequest. The same gap was tracked for the
229+
// multisig variant; the policy variant has the identical issue.
230+
if err := verifySpendTxMatchesRequest(context.Context(), tx, a.request); err != nil {
231+
return nil, errors.Wrap(err, "rejected spend transaction")
232+
}
226233
if _, err = context.RunView(ttx.NewEndorseView(tx)); err != nil {
227234
return nil, errors.Wrap(err, "failed to endorse transaction")
228235
}
229236

230237
return tx, nil
231238
}
239+
240+
// verifySpendTxMatchesRequest fails if the received transaction does not consume
241+
// exactly the token referenced by the SpendRequest.
242+
func verifySpendTxMatchesRequest(ctx context.Context, tx *ttx.Transaction, request *SpendRequest) error {
243+
if request == nil || request.Token == nil {
244+
return errors.New("spend request is missing the token to authorize")
245+
}
246+
record, err := tx.Request().AuditRecord(ctx)
247+
if err != nil {
248+
return errors.Wrap(err, "failed to extract audit record from transaction")
249+
}
250+
251+
return verifyInputIDsMatchExpected(record.Inputs.IDs(), request.Token.Id)
252+
}
253+
254+
// verifyInputIDsMatchExpected returns nil when every entry in inputIDs equals expected.
255+
// It is split out from verifySpendTxMatchesRequest so the comparison rule can be
256+
// exercised in unit tests without constructing a full ttx.Transaction.
257+
func verifyInputIDsMatchExpected(inputIDs []*token.ID, expected token.ID) error {
258+
if len(inputIDs) == 0 {
259+
return errors.Errorf("transaction has no inputs to validate against approved token [%s]", expected)
260+
}
261+
for _, id := range inputIDs {
262+
if id == nil || !id.Equal(expected) {
263+
return errors.Errorf(
264+
"transaction does not match approved spend request: expected token [%s], got input [%s]",
265+
expected, id,
266+
)
267+
}
268+
}
269+
270+
return nil
271+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
Copyright IBM Corp. All Rights Reserved.
3+
4+
SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package boolpolicy
8+
9+
import (
10+
"testing"
11+
12+
"github.com/hyperledger-labs/fabric-token-sdk/token/token"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestVerifyInputIDsMatchExpected(t *testing.T) {
18+
expected := token.ID{TxId: "tx-1", Index: 0}
19+
other := token.ID{TxId: "tx-2", Index: 0}
20+
21+
t.Run("single matching input passes", func(t *testing.T) {
22+
err := verifyInputIDsMatchExpected([]*token.ID{{TxId: "tx-1", Index: 0}}, expected)
23+
require.NoError(t, err)
24+
})
25+
26+
t.Run("single mismatched input is rejected", func(t *testing.T) {
27+
err := verifyInputIDsMatchExpected([]*token.ID{&other}, expected)
28+
require.Error(t, err)
29+
assert.Contains(t, err.Error(), "does not match approved spend request")
30+
})
31+
32+
t.Run("any mismatch in a set is rejected", func(t *testing.T) {
33+
err := verifyInputIDsMatchExpected([]*token.ID{
34+
{TxId: "tx-1", Index: 0},
35+
&other,
36+
}, expected)
37+
require.Error(t, err)
38+
})
39+
40+
t.Run("nil input id is rejected", func(t *testing.T) {
41+
err := verifyInputIDsMatchExpected([]*token.ID{nil}, expected)
42+
require.Error(t, err)
43+
})
44+
45+
t.Run("empty input list is rejected", func(t *testing.T) {
46+
err := verifyInputIDsMatchExpected(nil, expected)
47+
require.Error(t, err)
48+
assert.Contains(t, err.Error(), "no inputs")
49+
})
50+
51+
t.Run("differing index alone is rejected", func(t *testing.T) {
52+
err := verifyInputIDsMatchExpected(
53+
[]*token.ID{{TxId: "tx-1", Index: 1}},
54+
token.ID{TxId: "tx-1", Index: 0},
55+
)
56+
require.Error(t, err)
57+
})
58+
}

token/services/ttx/multisig/spend.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ SPDX-License-Identifier: Apache-2.0
77
package multisig
88

99
import (
10+
"context"
1011
"slices"
1112
"time"
1213

@@ -251,7 +252,13 @@ func (a *EndorseSpendView) Call(context view.Context) (interface{}, error) {
251252
}
252253
logger.DebugfContext(context.Context(), "multisig tx received with id [%s]", tx.ID())
253254

254-
// TODO: check tx matches request
255+
// Verify the received transaction actually consumes the token this co-owner
256+
// approved. Without this check, a malicious or buggy initiator could obtain
257+
// approval for one token and submit a transaction spending a different one,
258+
// breaking the multisig consent property.
259+
if err := verifySpendTxMatchesRequest(context.Context(), tx, a.request); err != nil {
260+
return nil, errors.Wrap(err, "rejected spend transaction")
261+
}
255262

256263
// If everything is fine, the recipient accepts and sends back her signature.
257264
// Notice that, a signature from the recipient might or might not be required to make the transaction valid.
@@ -266,3 +273,36 @@ func (a *EndorseSpendView) Call(context view.Context) (interface{}, error) {
266273

267274
return tx, nil
268275
}
276+
277+
// verifySpendTxMatchesRequest fails if the received transaction does not consume
278+
// exactly the token referenced by the SpendRequest.
279+
func verifySpendTxMatchesRequest(ctx context.Context, tx *ttx.Transaction, request *SpendRequest) error {
280+
if request == nil || request.Token == nil {
281+
return errors.New("spend request is missing the token to authorize")
282+
}
283+
record, err := tx.Request().AuditRecord(ctx)
284+
if err != nil {
285+
return errors.Wrap(err, "failed to extract audit record from transaction")
286+
}
287+
288+
return verifyInputIDsMatchExpected(record.Inputs.IDs(), request.Token.Id)
289+
}
290+
291+
// verifyInputIDsMatchExpected returns nil when every entry in inputIDs equals expected.
292+
// It is split out from verifySpendTxMatchesRequest so the comparison rule can be
293+
// exercised in unit tests without constructing a full ttx.Transaction.
294+
func verifyInputIDsMatchExpected(inputIDs []*token.ID, expected token.ID) error {
295+
if len(inputIDs) == 0 {
296+
return errors.Errorf("transaction has no inputs to validate against approved token [%s]", expected)
297+
}
298+
for _, id := range inputIDs {
299+
if id == nil || !id.Equal(expected) {
300+
return errors.Errorf(
301+
"transaction does not match approved spend request: expected token [%s], got input [%s]",
302+
expected, id,
303+
)
304+
}
305+
}
306+
307+
return nil
308+
}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*
2+
Copyright IBM Corp. All Rights Reserved.
3+
4+
SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package multisig
8+
9+
import (
10+
"testing"
11+
12+
"github.com/hyperledger-labs/fabric-token-sdk/token/token"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestVerifyInputIDsMatchExpected(t *testing.T) {
18+
expected := token.ID{TxId: "tx-1", Index: 0}
19+
other := token.ID{TxId: "tx-2", Index: 0}
20+
21+
t.Run("single matching input passes", func(t *testing.T) {
22+
err := verifyInputIDsMatchExpected([]*token.ID{{TxId: "tx-1", Index: 0}}, expected)
23+
require.NoError(t, err)
24+
})
25+
26+
t.Run("multiple matching inputs pass", func(t *testing.T) {
27+
err := verifyInputIDsMatchExpected([]*token.ID{
28+
{TxId: "tx-1", Index: 0},
29+
{TxId: "tx-1", Index: 0},
30+
}, expected)
31+
require.NoError(t, err)
32+
})
33+
34+
t.Run("single mismatched input is rejected", func(t *testing.T) {
35+
err := verifyInputIDsMatchExpected([]*token.ID{&other}, expected)
36+
require.Error(t, err)
37+
assert.Contains(t, err.Error(), "does not match approved spend request")
38+
})
39+
40+
t.Run("any mismatch in a set is rejected", func(t *testing.T) {
41+
err := verifyInputIDsMatchExpected([]*token.ID{
42+
{TxId: "tx-1", Index: 0},
43+
&other,
44+
}, expected)
45+
require.Error(t, err)
46+
})
47+
48+
t.Run("nil input id is rejected", func(t *testing.T) {
49+
err := verifyInputIDsMatchExpected([]*token.ID{nil}, expected)
50+
require.Error(t, err)
51+
})
52+
53+
t.Run("empty input list is rejected", func(t *testing.T) {
54+
err := verifyInputIDsMatchExpected(nil, expected)
55+
require.Error(t, err)
56+
assert.Contains(t, err.Error(), "no inputs")
57+
})
58+
59+
t.Run("differing index alone is rejected", func(t *testing.T) {
60+
err := verifyInputIDsMatchExpected(
61+
[]*token.ID{{TxId: "tx-1", Index: 1}},
62+
token.ID{TxId: "tx-1", Index: 0},
63+
)
64+
require.Error(t, err)
65+
})
66+
}

0 commit comments

Comments
 (0)