Pitfall

Parties' Shares Not Validated as Non-Zero and Distinct

What can go wrong. Many MPC protocols build upon Shamir secret sharing, a $(t, n)$-threshold scheme that recovers a secret $s = f(0)$ from $t$ shares of a sharing polynomial $f(x) = s + \sum_{i=1}^{t-1} a_i x^i$ over $\mathbb{Z}_q$, with coefficients $a_i$ drawn uniformly at random. Each party $P_i$ holds the share $(i, y_i = f(i))$, and any $t$ parties can reconstruct via $s = \sum_{j} y_j \, l_j(0)$ with Lagrange basis $l_j(0) = \prod_{k, k \ne j} \frac{i_k}{i_k - i_j}$, where each $i_k$ is a party’s index (the evaluation point). Both the index $i$ and the share $x_i$ live in $\mathbb{Z}_q$, so every implementation must reduce modulo $q$ before using them. Two related failures arise when this reduction is skipped at the input boundary. First, if a party can choose its own index and the implementation rejects only the integer $0 \in \mathbb{Z}$, an attacker submitting $i = q$ (or any $k \cdot q$) passes the check while evaluatePolynomial(q) ≡ evaluatePolynomial(0) = f(0) = secret, handing it the secret directly. Second, the Lagrange basis denominator $i_k - i_j$ vanishes modulo $q$ whenever any two reconstruction indices coincide mod $q$, whether as the same raw integer (naïve duplicate) or as a malicious $i_k' = i_j + q$ (distinct as big.Int, congruent in $\mathbb{Z}_q$). The subsequent modular inverse is undefined.

Security implication. A party whose index reduces to $0 \bmod q$ is handed $f(0)$, the shared secret itself: the dealer evaluates the sharing polynomial at the attacker’s index and returns the result as normal. In a DKG, where every party deals a contribution, the attacker collects $f(0)$ from each dealer and reconstructs the full private key with no further interaction. The duplicate failure splits into two outcomes. In availability terms, reconstruction crashes with a nil-pointer dereference (Go’s ModInverse returns nil for a non-invertible input) or throws an unrecoverable error, DoS-ing the signing ceremony. In integrity terms, some implementations silently skip the offending term or substitute a default, producing an incorrect reconstruction the caller accepts as valid.

How to avoid. Validate indices at the protocol’s share-ingestion boundary: reduce each index modulo $q$, reject zero, and verify pairwise distinctness in a single pass.

Example tss-lib Shamir validation (Trail of Bits Shamir disclosure, PR #149)

Both failures appear in bnb-chain/tss-lib’s crypto/vss/feldman_vss.go and were disclosed together by Trail of Bits. They were fixed in a single PR #149.

Failure 1: zero index mod $q$. Before the fix, Create checked the party index against the integer literal 0 without reducing modulo $q$ first (source):

1// crypto/vss/feldman_vss.go, bnb-chain/tss-lib (vulnerable, pre-PR #149)
2for i := 0; i < num; i++ {
3    if indexes[i].Cmp(big.NewInt(0)) == 0 {
4        return nil, nil, fmt.Errorf("party index should not be 0")
5    }
6    // indexes[i] == q passes the check; evaluatePolynomial(q) ≡ f(0) = secret
7    share := evaluatePolynomial(ec, threshold, poly, indexes[i])
8    shares[i] = &Share{Threshold: threshold, ID: indexes[i], Share: share}
9}

A malicious party submits index $i = q$. The literal-zero check passes, but evaluatePolynomial(q) ≡ evaluatePolynomial(0) = f(0) = s, handing the attacker the shared secret as their share.

Failure 2: duplicate indices mod $q$. The same file’s ReConstruct performs Lagrange interpolation by inverting the index difference $x_j - x_k$ via ModInverse (source):

1// crypto/vss/feldman_vss.go, bnb-chain/tss-lib (Lagrange step in ReConstruct)
2sub := modN.Sub(xs[j], share.ID)
3subInv := modN.ModInverse(sub)         // nil if sub ≡ 0 mod q
4div := modN.Mul(xs[j], subInv)         // nil-pointer dereference
5times = modN.Mul(times, div)

A malicious party submits $x_j = x_k + q$ for some honest party $k$. The raw integers differ, so any non-modular != check passes; modular reduction makes $x_j \equiv x_k$, sub is zero, ModInverse returns nil, and the next operation panics, DoS-ing the signing ceremony.

Unified fix: CheckIndexes. PR #149 added a single validation helper called at the start of Create. It reduces each index modulo $q$, rejects zero, and rejects duplicates in one pass (source):

 1// crypto/vss/feldman_vss.go, bnb-chain/tss-lib (fixed, PR #149)
 2func CheckIndexes(ec elliptic.Curve, indexes []*big.Int) ([]*big.Int, error) {
 3    visited := make(map[string]struct{})
 4    for _, v := range indexes {
 5        vMod := new(big.Int).Mod(v, ec.Params().N)
 6        if vMod.Cmp(zero) == 0 {
 7            return nil, errors.New("party index should not be 0")
 8        }
 9        vModStr := vMod.String()
10        if _, ok := visited[vModStr]; ok {
11            return nil, fmt.Errorf("duplicate indexes %s", vModStr)
12        }
13        visited[vModStr] = struct{}{}
14    }
15    return indexes, nil
16}