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

Open Beginner friendly
#238 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
75/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
go
Domain
tooling

Research direction

Start in tools/fxconfig/internal/transaction/merge.go, reading validateTransactionsForMerge and mergeEndorsements to trace how namespace and endorsement counts are handled. Run the reproduction with go test ./tools/fxconfig/internal/transaction -run TestMergePanicsWhenEndorsementsExceedNamespaces -v; done means Merge returns a descriptive error instead of panicking for mismatched counts.

Written by the indexing model from the issue text.

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

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/:

// 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:

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:

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+
Dominant language
Go
Stars
64
Forks
81
Avg merge
1d 22h
Merged PRs (30d)
15

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from hyperledger/fabric-x

All issues in hyperledger/fabric-x

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.