hyperledger / hyperledger/fabric-x

bug(fxconfig/transaction): Merge panics with index out of range when Endorsements count exceeds Namespaces count

Open
#238 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
64
Forks
80
Avg merge
1d 22h
Merged PRs (30d)
15

Description

## Summary

`mergeEndorsements()` allocates its internal slices using `len(txs[0].GetNamespaces())`,
but then indexes into those slices using positions from `tx.GetEndorsements()`.
When any transaction carries more endorsement entries than namespace entries,
this causes a **runtime panic: index out of range**.

## Affected File

`tools/fxconfig/internal/transaction/merge.go`

## Root Cause

```go
func mergeEndorsements(txs []*applicationpb.Tx) []*applicationpb.Endorsements {
numNamespaces := len(txs.GetNamespaces()) // ← allocated from Namespaces count
merged := make([]*applicationpb.Endorsements, numNamespaces)
seen := make([]map[string]struct{}, numNamespaces)
...
for _, tx := range txs {
for nsIdx, ns := range tx.GetEndorsements() { // ← iterates Endorsements (different length!)
...
seen[nsIdx][key] = struct{}{} // ← PANIC if nsIdx >= numNamespaces
merged[nsIdx].EndorsementsWithIdentity = append(...)
}
}
}
```

`validateTransactionsForMerge` only validates:
- Namespace count is the same across all transactions (`len(Namespaces)` matches)
- Each endorsement entry is non-empty

It does **not** validate that `len(Endorsements) == len(Namespaces)` within each
individual transaction. A transaction with 1 namespace but 2 endorsement slots passes
validation and then panics inside `mergeEndorsements`.

## Steps to Reproduce

Add the following test to `tools/fxconfig/internal/transaction/`:

```go
// merge_panic_test.go
package transaction

import (
"testing"

"github.com/hyperledger/fabric-x-common/api/applicationpb"
"github.com/hyperledger/fabric-x-common/api/msppb"
)

func TestMergePanicsWhenEndorsementsExceedNamespaces(t *testing.T) {
tx1 := &applicationpb.Tx{
Namespaces: []*applicationpb.TxNamespace{
{NsId: "ns1", NsVersion: 1},
},
Endorsements: []*applicationpb.Endorsements{
{
EndorsementsWithIdentity: []*applicationpb.EndorsementWithIdentity{
{Endorsement: []byte("sig-a1"), Identity: &msppb.Identity{MspId: "Org1MSP"}},
},
},
// Extra endorsement entry — no matching namespace
{
EndorsementsWithIdentity: []*applicationpb.EndorsementWithIdentity{
{Endorsement: []byte("sig-a2"), Identity: &msppb.Identity{MspId: "Org2MSP"}},
},
},
},
}

tx2 := &applicationpb.Tx{
Namespaces: []*applicationpb.TxNamespace{
{NsId: "ns1", NsVersion: 1},
},
Endorsements: []*applicationpb.Endorsements{
{
EndorsementsWithIdentity: []*applicationpb.EndorsementWithIdentity{
{Endorsement: []byte("sig-b1"), Identity: &msppb.Identity{MspId: "Org3MSP"}},
},
},
{
EndorsementsWithIdentity: []*applicationpb.EndorsementWithIdentity{
{Endorsement: []byte("sig-b2"), Identity: &msppb.Identity{MspId: "Org4MSP"}},
},
},
},
}

defer func() {
if r := recover(); r == nil {
t.Fatalf("expected panic due to index out of range, got nil")
}
}()

_, _ = Merge([]*applicationpb.Tx{tx1, tx2})
}
```

Run:

```bash
go test ./tools/fxconfig/internal/transaction -run TestMergePanicsWhenEndorsementsExceedNamespaces -v
```

## Observed Behavior
--- PASS: TestMergePanicsWhenEndorsementsExceedNamespaces (0.00s)
PASS

The test **passes** (panic is caught by `recover()`), confirming the runtime panic occurs.

## Expected Behavior

`Merge()` should return a descriptive error instead of panicking. Either:
1. `validateTransactionsForMerge` should reject transactions where `len(Endorsements) != len(Namespaces)`, **or**
2. `mergeEndorsements` should guard the index with a bounds check.

## Suggested Fix

Add this check inside `validateTransactionsForMerge`:

```go
for i, tx := range txs {
if len(tx.GetEndorsements()) != len(tx.GetNamespaces()) {
return fmt.Errorf(
"transaction %d: endorsements count (%d) does not match namespaces count (%d)",
i, len(tx.GetEndorsements()), len(tx.GetNamespaces()),
)
}
// ... existing checks
}
```

## Environment

- Repo: `hyperledger/fabric-x`
- File: `tools/fxconfig/internal/transaction/merge.go`
- Function: `mergeEndorsements` / `validateTransactionsForMerge`
- Go version: tested on `go1.23+`

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.