Skip to content

Commit e63e6ea

Browse files
committed
fix(idemix): reject out-of-range curve ids in audit info
mathlib indexes its curve table with the curve id found in the payload without a bounds check, so audit info JSON carrying an out-of-range id panicked during json.Unmarshal. Run both audit info decodes under a recover that turns it into an error, as FromG1Proto already does. Signed-off-by: AkramBitar <akram@il.ibm.com>
1 parent 4d5a067 commit e63e6ea

8 files changed

Lines changed: 455 additions & 14 deletions

File tree

docs/services/identity.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,45 @@ An extension of Idemix that uses a **commitment to the Enrollment ID (EID)** as
296296
| **Identity Size** | Large (~several KB) | Small (~32-64 bytes) |
297297
| **Storage Overhead** | High | Low |
298298

299+
#### Audit Info Deserialization (Idemix and IdemixNym)
300+
301+
Audit info is JSON and can arrive from a counterparty (recipient registration, auditing
302+
flows), so both `crypto.AuditInfo.FromBytes`
303+
(`token/services/identity/idemix/crypto/audit.go`) and `nym.AuditInfo.FromBytes`
304+
(`token/services/identity/idemixnym/nym/audit.go`) treat their input as untrusted and reject
305+
malformed payloads with an error.
306+
307+
`EidNymAuditData` and `RhNymAuditData` embed `mathlib` curve elements, which JSON-encode as
308+
a curve ID plus the raw element bytes:
309+
310+
```json
311+
{"EidNymAuditData":{"Nym":{"curve":3,"element":"..."},"Rand":{...},"Attr":{...}}}
312+
```
313+
314+
`mathlib`'s `UnmarshalJSON` uses that curve ID to index its internal curve table **without a
315+
bounds check**, so an out-of-range ID raises an `index out of range` panic from inside
316+
`encoding/json`. Both `FromBytes` implementations therefore run their decode through
317+
`crypto.UnmarshalAuditInfo`, which recovers that panic and returns it as an ordinary error:
318+
319+
```go
320+
return crypto.UnmarshalAuditInfo(func() error {
321+
return json.Unmarshal(raw, a)
322+
})
323+
```
324+
325+
The guard wraps the real decode rather than pre-validating the payload's curve IDs, because
326+
`mathlib` runs *during* `encoding/json`'s traversal: a separate validation pass has to
327+
reproduce that traversal exactly to see every curve element the decode reaches, including
328+
ones that never appear in the decoded result (a duplicate key overwriting an earlier value,
329+
input after the first JSON value, a curve element following a type error). The same defect
330+
is contained the same way in `FromG1Proto`
331+
(`token/core/zkatdlog/nogh/protos-go/utils/proto.go`).
332+
333+
Where curve IDs arrive as plain data rather than through a third-party unmarshaler, prefer an
334+
explicit bounds check instead — see `curveAt` in
335+
`token/core/common/encoding/asn1/asn1.go` and `PublicParams.Validate` in
336+
`token/core/zkatdlog/nogh/v1/setup/setup.go`.
337+
299338
### Other Identity Types
300339

301340
The architecture supports specialized identity types for complex use cases:

token/services/identity/idemix/crypto/audit.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ func (a *AuditInfo) Bytes() ([]byte, error) {
4545

4646
// FromBytes deserializes the AuditInfo from JSON format.
4747
func (a *AuditInfo) FromBytes(raw []byte) error {
48-
return json.Unmarshal(raw, a)
48+
// raw is untrusted and the pseudonym audit data holds mathlib curve elements,
49+
// which panic on an out-of-range curve ID instead of rejecting it.
50+
return UnmarshalAuditInfo(func() error {
51+
return json.Unmarshal(raw, a)
52+
})
4953
}
5054

5155
// EnrollmentID returns the enrollment ID from Attributes[2].

token/services/identity/idemix/crypto/audit_fuzz_test.go

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ SPDX-License-Identifier: Apache-2.0
77
package crypto
88

99
import (
10+
"strconv"
1011
"testing"
1112

1213
idemix "github.com/IBM/idemix/bccsp/types"
14+
math "github.com/IBM/mathlib"
1315
"github.com/stretchr/testify/require"
1416
)
1517

@@ -23,13 +25,14 @@ const maxFuzzAuditInfoBytes = 64 << 10
2325
// indexed into Attributes, and Match dereferenced EidNymAuditData/
2426
// RhNymAuditData without nil checks).
2527
func FuzzDeserializeAuditInfoNoPanic(f *testing.F) {
28+
attributes := [][]byte{
29+
[]byte("attr0"),
30+
[]byte("attr1"),
31+
[]byte("enrollment-id"),
32+
[]byte("revocation-handle"),
33+
}
2634
valid := &AuditInfo{
27-
Attributes: [][]byte{
28-
[]byte("attr0"),
29-
[]byte("attr1"),
30-
[]byte("enrollment-id"),
31-
[]byte("revocation-handle"),
32-
},
35+
Attributes: attributes,
3336
Schema: "test-schema",
3437
EidNymAuditData: &idemix.AttrNymAuditData{},
3538
RhNymAuditData: &idemix.AttrNymAuditData{},
@@ -43,6 +46,40 @@ func FuzzDeserializeAuditInfoNoPanic(f *testing.F) {
4346
f.Add([]byte(`{"Attributes":[[0],[1],[101,105,100],[114,104]],"Schema":""}`))
4447
f.Add([]byte(`{"Attributes":[[0]]}`))
4548

49+
// The seeds above leave the pseudonym audit data zero-valued, which marshals
50+
// its mathlib fields as JSON null and so never reaches their UnmarshalJSON.
51+
// Seed populated curve elements too, along with a curve ID no curve is
52+
// registered under: mathlib indexes math.Curves with that ID unchecked, so
53+
// this is the input class that used to panic instead of being rejected.
54+
curve := math.Curves[math.BLS12_381]
55+
populated := &AuditInfo{
56+
Attributes: attributes,
57+
Schema: "test-schema",
58+
EidNymAuditData: &idemix.AttrNymAuditData{
59+
Nym: curve.GenG1,
60+
Rand: curve.NewZrFromInt(7),
61+
Attr: curve.NewZrFromInt(11),
62+
},
63+
RhNymAuditData: &idemix.AttrNymAuditData{
64+
Nym: curve.GenG1,
65+
Rand: curve.NewZrFromInt(13),
66+
Attr: curve.NewZrFromInt(17),
67+
},
68+
}
69+
populatedBytes, err := populated.Bytes()
70+
require.NoError(f, err)
71+
f.Add(populatedBytes)
72+
f.Add([]byte(`{"EidNymAuditData":{"Nym":{"curve":999999,"element":"AQID"}}}`))
73+
f.Add([]byte(`{"EidNymAuditData":{"Rand":{"curve":-1,"element":"AQID"}}}`))
74+
f.Add([]byte(`{"RhNymAuditData":{"Attr":{"curve":` + strconv.Itoa(len(math.Curves)) + `,"element":"AQID"}}}`))
75+
// Out-of-range curve ids that a pre-validating version of the guard failed to
76+
// see, because encoding/json reaches them but they do not survive into the
77+
// decoded result. See TestDeserializeAuditInfoOutOfRangeCurveIDEvasions.
78+
f.Add([]byte(`{"EidNymAuditData":5,"RhNymAuditData":{"Nym":{"curve":999999,"element":"AQID"}}}`))
79+
f.Add([]byte(`{"RhNymAuditData":{"Attr":{"curve":9}}}0`))
80+
f.Add([]byte(`{"EidNymAuditData":{"Nym":{"curve":999999,"element":"AQID"},"Nym":null}}`))
81+
f.Add([]byte(`{"eidnymauditdata":{"nym":{"CURVE":999999,"element":"AQID"}}}`))
82+
4683
f.Fuzz(func(t *testing.T, raw []byte) {
4784
if len(raw) > maxFuzzAuditInfoBytes {
4885
t.Skip()
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
Copyright IBM Corp. All Rights Reserved.
3+
4+
SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package crypto
8+
9+
import (
10+
"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
11+
)
12+
13+
// UnmarshalAuditInfo runs decode, converting a panic raised while decoding into
14+
// an ordinary error.
15+
//
16+
// Audit info holds mathlib curve elements, whose UnmarshalJSON indexes mathlib's
17+
// internal curve table with the "curve" field taken straight from the payload,
18+
// without bounds-checking it (see marshaler.go in github.com/IBM/mathlib) — so an
19+
// out-of-range curve ID panics instead of being rejected. mathlib v0.3.0 is the
20+
// newest published version, so this has to be contained here. The same defect is
21+
// contained the same way in FromG1Proto, in
22+
// token/core/zkatdlog/nogh/protos-go/utils.
23+
//
24+
// The recover deliberately wraps the real decode rather than pre-validating the
25+
// payload: mathlib runs during encoding/json's traversal, and a separate
26+
// validation pass would have to reproduce that traversal exactly to see every
27+
// curve element it decodes — including ones a later duplicate key overwrites,
28+
// which never appear in the decoded result at all.
29+
func UnmarshalAuditInfo(decode func() error) (err error) {
30+
defer func() {
31+
if r := recover(); r != nil {
32+
err = errors.Errorf("failed to unmarshal audit info: caught panic [%v]", r)
33+
}
34+
}()
35+
36+
return decode()
37+
}
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/*
2+
Copyright IBM Corp. All Rights Reserved.
3+
4+
SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package crypto
8+
9+
import (
10+
"strconv"
11+
"testing"
12+
13+
csp "github.com/IBM/idemix/bccsp/types"
14+
math "github.com/IBM/mathlib"
15+
"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
16+
"github.com/stretchr/testify/assert"
17+
"github.com/stretchr/testify/require"
18+
)
19+
20+
// auditInfoWithCurveID returns audit info JSON whose EID pseudonym audit data
21+
// carries the given curve ID in the named field. The curve ID goes in verbatim,
22+
// so it can be a value no curve is registered under.
23+
func auditInfoWithCurveID(field string, curveID int) []byte {
24+
return []byte(`{"EidNymAuditData":{"` + field + `":{"curve":` +
25+
strconv.Itoa(curveID) + `,"element":"AQID"}},"Schema":"test-schema"}`)
26+
}
27+
28+
// TestDeserializeAuditInfoOutOfRangeCurveID covers the curve IDs no curve is
29+
// registered under. mathlib uses each of them to index its curve table, so before
30+
// the guard every one of these payloads killed the process with an
31+
// "index out of range" panic rather than being rejected.
32+
func TestDeserializeAuditInfoOutOfRangeCurveID(t *testing.T) {
33+
for _, curveID := range []int{-1, len(math.Curves), len(math.Curves) + 1, 999999} {
34+
for _, field := range []string{"Nym", "Rand", "Attr"} {
35+
t.Run(field+"/"+strconv.Itoa(curveID), func(t *testing.T) {
36+
raw := auditInfoWithCurveID(field, curveID)
37+
38+
var err error
39+
require.NotPanics(t, func() {
40+
_, err = DeserializeAuditInfo(raw)
41+
})
42+
require.Error(t, err)
43+
44+
require.NotPanics(t, func() {
45+
err = (&AuditInfo{}).FromBytes(raw)
46+
})
47+
require.Error(t, err)
48+
})
49+
}
50+
}
51+
}
52+
53+
// TestDeserializeAuditInfoOutOfRangeCurveIDEvasions collects payloads that hid an
54+
// out-of-range curve ID from an earlier version of this guard, which pre-validated
55+
// the raw bytes with its own decode pass instead of wrapping the real one. Each
56+
// exploits a way that pass diverged from what encoding/json actually traverses,
57+
// and each panicked while the pre-validation reported the payload as clean.
58+
func TestDeserializeAuditInfoOutOfRangeCurveIDEvasions(t *testing.T) {
59+
for name, raw := range map[string]string{
60+
// json.Unmarshal validates the whole input up front and decodes nothing on
61+
// a syntax error, but FromBytes' decoder reads one value and ignores what
62+
// follows it. Found by FuzzDeserializeAuditInfoNoPanic.
63+
"trailing garbage": `{"RhNymAuditData":{"Attr":{"curve":9}}}0`,
64+
// mathlib's UnmarshalJSON runs on every occurrence of a key, so the first
65+
// one panics even though only the last survives in the decoded result.
66+
"duplicate outer key": `{"EidNymAuditData":{"Nym":{"curve":999999,"element":"AQID"}},` +
67+
`"EidNymAuditData":{"Nym":null}}`,
68+
"duplicate inner key": `{"EidNymAuditData":{"Nym":{"curve":999999,"element":"AQID"},"Nym":null}}`,
69+
// encoding/json records a type error and keeps decoding, so the curve
70+
// element after it is still reached.
71+
"type error first": `{"EidNymAuditData":5,"RhNymAuditData":{"Nym":{"curve":999999,"element":"AQID"}}}`,
72+
// Key matching is case-insensitive.
73+
"unexpected key casing": `{"eidnymauditdata":{"nym":{"CURVE":999999,"element":"AQID"}}}`,
74+
} {
75+
t.Run(name, func(t *testing.T) {
76+
var err error
77+
require.NotPanics(t, func() {
78+
_, err = DeserializeAuditInfo([]byte(raw))
79+
})
80+
require.Error(t, err)
81+
})
82+
}
83+
}
84+
85+
// TestUnmarshalAuditInfo covers the guard on its own: a decode that panics becomes
86+
// an error naming the panic, and anything else is passed through untouched.
87+
func TestUnmarshalAuditInfo(t *testing.T) {
88+
err := UnmarshalAuditInfo(func() error {
89+
panic("boom")
90+
})
91+
require.Error(t, err)
92+
assert.Contains(t, err.Error(), "caught panic")
93+
assert.Contains(t, err.Error(), "boom")
94+
95+
sentinel := errors.New("decode failed")
96+
assert.Equal(t, sentinel, UnmarshalAuditInfo(func() error { return sentinel }))
97+
require.NoError(t, UnmarshalAuditInfo(func() error { return nil }))
98+
}
99+
100+
// TestAuditInfoPopulatedCurveElementsRoundTrip makes sure the guard does not get
101+
// in the way of audit info that actually carries curve elements — the case the
102+
// fuzz seed corpus used to miss, since a zero-value AttrNymAuditData marshals its
103+
// mathlib fields as JSON null and never reaches their UnmarshalJSON.
104+
func TestAuditInfoPopulatedCurveElementsRoundTrip(t *testing.T) {
105+
for curveID := range math.Curves {
106+
t.Run(strconv.Itoa(curveID), func(t *testing.T) {
107+
curve := math.Curves[curveID]
108+
auditData := func() *csp.AttrNymAuditData {
109+
return &csp.AttrNymAuditData{
110+
Nym: curve.GenG1,
111+
Rand: curve.NewZrFromInt(7),
112+
Attr: curve.NewZrFromInt(11),
113+
}
114+
}
115+
auditInfo := &AuditInfo{
116+
Attributes: [][]byte{
117+
[]byte("attr0"),
118+
[]byte("attr1"),
119+
[]byte("enrollment-id"),
120+
[]byte("revocation-handle"),
121+
},
122+
Schema: "test-schema",
123+
EidNymAuditData: auditData(),
124+
RhNymAuditData: auditData(),
125+
}
126+
raw, err := auditInfo.Bytes()
127+
require.NoError(t, err)
128+
129+
deserialized, err := DeserializeAuditInfo(raw)
130+
require.NoError(t, err)
131+
assert.Equal(t, auditInfo.EnrollmentID(), deserialized.EnrollmentID())
132+
assert.Equal(t, auditInfo.RevocationHandle(), deserialized.RevocationHandle())
133+
assert.True(t, auditInfo.EidNymAuditData.Nym.Equals(deserialized.EidNymAuditData.Nym))
134+
assert.True(t, auditInfo.EidNymAuditData.Rand.Equals(deserialized.EidNymAuditData.Rand))
135+
assert.True(t, auditInfo.RhNymAuditData.Attr.Equals(deserialized.RhNymAuditData.Attr))
136+
})
137+
}
138+
}
139+
140+
// TestDeserializeAuditInfoInRangeCurveIDAccepted checks the guard does not turn
141+
// every curve ID into an error: the boundary IDs a curve is registered under are
142+
// still decoded.
143+
func TestDeserializeAuditInfoInRangeCurveIDAccepted(t *testing.T) {
144+
for _, curveID := range []int{0, len(math.Curves) - 1} {
145+
t.Run(strconv.Itoa(curveID), func(t *testing.T) {
146+
curve := math.Curves[curveID]
147+
raw, err := (&AuditInfo{
148+
Attributes: [][]byte{{0}, {1}, []byte("eid"), []byte("rh")},
149+
EidNymAuditData: &csp.AttrNymAuditData{
150+
Nym: curve.GenG1, Rand: curve.NewZrFromInt(1), Attr: curve.NewZrFromInt(2),
151+
},
152+
RhNymAuditData: &csp.AttrNymAuditData{
153+
Nym: curve.GenG1, Rand: curve.NewZrFromInt(3), Attr: curve.NewZrFromInt(4),
154+
},
155+
}).Bytes()
156+
require.NoError(t, err)
157+
158+
ai, err := DeserializeAuditInfo(raw)
159+
require.NoError(t, err)
160+
assert.Equal(t, curveID, int(ai.EidNymAuditData.Nym.CurveID()))
161+
})
162+
}
163+
}

token/services/identity/idemixnym/nym/audit.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,12 @@ type AuditInfo struct {
2222

2323
// FromBytes deserializes the AuditInfo from JSON format.
2424
func (a *AuditInfo) FromBytes(raw []byte) error {
25-
return json.Unmarshal(raw, a)
25+
// The embedded crypto.AuditInfo's fields are inlined into this struct's JSON,
26+
// so this decode reaches the panicking mathlib curve elements itself rather
27+
// than going through crypto.AuditInfo.FromBytes. Guard it here too.
28+
return crypto.UnmarshalAuditInfo(func() error {
29+
return json.Unmarshal(raw, a)
30+
})
2631
}
2732

2833
func (a *AuditInfo) Match(ctx context.Context, id []byte) error {

0 commit comments

Comments
 (0)