Skip to content

Commit 31097ce

Browse files
ETCatonETCaton
committed
Use cryptobyte for bounds checking
Go has a helpful /x/crypto/cryptobyte which includes bounds checking. Unfortunately, it was designed with protocols which mandate big-endian in mind, and these values are little-endian To avoid breaking all the ciphertexts out in the wild we compromise on cryptobyte for bounds checking then doing some little-endian bookkeeping ourselves, which still provides a relatively centralized place instead of a large amount of diffs
1 parent a07bb99 commit 31097ce

4 files changed

Lines changed: 235 additions & 201 deletions

File tree

‎abe/cpabe/tkn20/internal/tkn/formula.go‎

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
package tkn
22

33
import (
4-
"encoding/binary"
54
"fmt"
65
"io"
7-
"math"
6+
7+
"golang.org/x/crypto/cryptobyte"
88
)
99

1010
const (
@@ -61,40 +61,44 @@ func (g Gate) Equal(g2 Gate) bool {
6161

6262
func (f *Formula) MarshalBinary() ([]byte, error) {
6363
n := len(f.Gates)
64-
if n > math.MaxUint16 {
65-
return nil, fmt.Errorf("too many gates")
64+
b := cryptobyte.NewBuilder(nil)
65+
b.AddValue(leUint16(n))
66+
for _, gate := range f.Gates {
67+
b.AddUint8(byte(gate.Class))
68+
b.AddValue(leUint16(gate.In0))
69+
b.AddValue(leUint16(gate.In1))
70+
b.AddValue(leUint16(gate.Out))
6671
}
67-
ret := make([]byte, 2+7*n)
68-
binary.LittleEndian.PutUint16(ret, uint16(len(f.Gates)))
69-
for i, gate := range f.Gates {
70-
if gate.In0 < 0 || gate.In0 > math.MaxUint16 ||
71-
gate.In1 < 0 || gate.In1 > math.MaxUint16 ||
72-
gate.Out < 0 || gate.Out > math.MaxUint16 {
73-
return nil, fmt.Errorf("wire index out of range")
74-
}
75-
76-
ret[7*i+2] = byte(gate.Class)
77-
binary.LittleEndian.PutUint16(ret[7*i+2+1:], uint16(gate.In0))
78-
binary.LittleEndian.PutUint16(ret[7*i+2+3:], uint16(gate.In1))
79-
binary.LittleEndian.PutUint16(ret[7*i+2+5:], uint16(gate.Out))
72+
ret, err := b.Bytes()
73+
if err != nil {
74+
return nil, err
8075
}
8176
return ret, nil
8277
}
8378

8479
func (f *Formula) UnmarshalBinary(data []byte) error {
85-
if len(data) < 2 {
86-
return fmt.Errorf("too short data")
87-
}
88-
n := int(binary.LittleEndian.Uint16(data[0:2]))
89-
if len(data) < 2+7*n {
80+
s := cryptobyte.String(data)
81+
n16, ok := readLEUint16(&s)
82+
if !ok {
9083
return fmt.Errorf("too short data")
9184
}
85+
n := int(n16)
9286
f.Gates = make([]Gate, n)
93-
for i := 0; i < n; i++ {
94-
f.Gates[i].Class = int(data[7*i+2])
95-
f.Gates[i].In0 = int(binary.LittleEndian.Uint16(data[7*i+2+1:]))
96-
f.Gates[i].In1 = int(binary.LittleEndian.Uint16(data[7*i+2+3:]))
97-
f.Gates[i].Out = int(binary.LittleEndian.Uint16(data[7*i+2+5:]))
87+
for i := range n {
88+
var class uint8
89+
if !s.ReadUint8(&class) {
90+
return fmt.Errorf("too short data")
91+
}
92+
in0, ok0 := readLEUint16(&s)
93+
in1, ok1 := readLEUint16(&s)
94+
out, ok2 := readLEUint16(&s)
95+
if !ok0 || !ok1 || !ok2 {
96+
return fmt.Errorf("too short data")
97+
}
98+
f.Gates[i].Class = int(class)
99+
f.Gates[i].In0 = int(in0)
100+
f.Gates[i].In1 = int(in1)
101+
f.Gates[i].Out = int(out)
98102
}
99103
// Reject malformed or cyclic gate graphs coming from untrusted encodings.
100104
check := Formula{Gates: append([]Gate(nil), f.Gates...)}

‎abe/cpabe/tkn20/internal/tkn/policy.go‎

Lines changed: 58 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
package tkn
22

33
import (
4-
"encoding/binary"
54
"fmt"
6-
"math"
75

86
pairing "github.com/cloudflare/circl/ecc/bls12381"
7+
"golang.org/x/crypto/cryptobyte"
98
)
109

1110
const (
@@ -75,29 +74,28 @@ func (a *Attribute) Equal(b *Attribute) bool {
7574
type Attributes map[string]Attribute
7675

7776
func (a *Attributes) marshalBinary() ([]byte, error) {
78-
if len(*a) > math.MaxUint16 {
79-
return nil, fmt.Errorf("too many attributes (overflow)")
80-
}
81-
ret := make([]byte, 2)
82-
binary.LittleEndian.PutUint16(ret[0:], uint16(len(*a)))
83-
8477
aBytes, err := marshalBinarySortedMapAttribute(*a)
8578
if err != nil {
8679
return nil, fmt.Errorf("marshalling Attributes failed: %w", err)
8780
}
88-
ret = append(ret, aBytes...)
89-
90-
return ret, nil
81+
b := cryptobyte.NewBuilder(nil)
82+
if err = leUint16(len(*a)).Marshal(b); err != nil {
83+
return nil, fmt.Errorf("too many attributes")
84+
}
85+
b.AddBytes(aBytes)
86+
return b.Bytes()
9187
}
9288

9389
func (a *Attributes) unmarshalBinary(data []byte) error {
94-
if len(data) < 2 {
90+
s := cryptobyte.String(data)
91+
n16, ok := readLEUint16(&s)
92+
if !ok {
9593
return fmt.Errorf("unmarshalling Attributes failed: data too short")
9694
}
97-
n := int(binary.LittleEndian.Uint16(data))
98-
data = data[2:]
95+
n := int(n16)
96+
data = s
9997
*a = make(map[string]Attribute, n)
100-
for i := 0; i < n; i++ {
98+
for range n {
10199
labelBytes, rem, err := removeLenPrefixed(data)
102100
if err != nil {
103101
return fmt.Errorf("unmarshalling Attributes failed: %w", err)
@@ -139,72 +137,48 @@ func (w *Wire) MarshalBinary() ([]byte, error) {
139137
if err != nil {
140138
return nil, err
141139
}
142-
if len(strBytes) > math.MaxUint16 || len(valBytes) > math.MaxUint16 || len(intBytes) > math.MaxUint16 {
143-
return nil, fmt.Errorf("data too long")
144-
}
145-
totalLen := len(strBytes) + len(valBytes) + len(intBytes) + 2 + 2 + 2 + 1
146-
ret := make([]byte, totalLen)
147-
where := 0
148-
binary.LittleEndian.PutUint16(ret[where:], uint16(len(strBytes)))
149-
where += 2
150-
where += copy(ret[where:], strBytes)
151-
binary.LittleEndian.PutUint16(ret[where:], uint16(len(valBytes)))
152-
where += 2
153-
where += copy(ret[where:], valBytes)
154-
binary.LittleEndian.PutUint16(ret[where:], uint16(len(intBytes)))
155-
where += 2
156-
where += copy(ret[where:], intBytes)
140+
b := cryptobyte.NewBuilder(nil)
141+
b.AddValue(lenPrefixed16(strBytes))
142+
b.AddValue(lenPrefixed16(valBytes))
143+
b.AddValue(lenPrefixed16(intBytes))
157144
if w.Positive {
158-
ret[where] = 1
145+
b.AddUint8(1)
159146
} else {
160-
ret[where] = 0
147+
b.AddUint8(0)
161148
}
162-
return ret, nil
149+
return b.Bytes()
163150
}
164151

165152
func (w *Wire) UnmarshalBinary(data []byte) error {
166-
where := 0
167-
if len(data) < 2 {
168-
return fmt.Errorf("data not long enough")
169-
}
170-
strLen := int(binary.LittleEndian.Uint16(data[where:]))
171-
where += 2
172-
if len(data[where:]) < strLen {
173-
return fmt.Errorf("data not long enough")
174-
}
175-
w.Label = string(data[where : where+strLen])
176-
where += strLen
153+
s := cryptobyte.String(data)
177154

178-
if len(data[where:]) < 2 {
179-
return fmt.Errorf("data not long enough")
180-
}
181-
valLen := int(binary.LittleEndian.Uint16(data[where:]))
182-
where += 2
183-
if len(data[where:]) < valLen {
155+
strLen, ok := readLEUint16(&s)
156+
var strBytes []byte
157+
if !ok || !s.ReadBytes(&strBytes, int(strLen)) {
184158
return fmt.Errorf("data not long enough")
185159
}
186-
w.RawValue = string(data[where : where+valLen])
187-
where += valLen
160+
w.Label = string(strBytes)
188161

189-
if len(data[where:]) < 2 {
162+
valLen, ok := readLEUint16(&s)
163+
var valBytes []byte
164+
if !ok || !s.ReadBytes(&valBytes, int(valLen)) {
190165
return fmt.Errorf("data not long enough")
191166
}
192-
intLen := int(binary.LittleEndian.Uint16(data[where:]))
193-
where += 2
194-
if len(data[where:]) < intLen {
167+
w.RawValue = string(valBytes)
168+
169+
intLen, ok := readLEUint16(&s)
170+
var intBytes []byte
171+
if !ok || !s.ReadBytes(&intBytes, int(intLen)) {
195172
return fmt.Errorf("data not long enough")
196173
}
197174
w.Value = &pairing.Scalar{}
198-
w.Value.SetBytes(data[where : where+intLen])
199-
where += intLen
200-
if len(data[where:]) < 1 {
175+
w.Value.SetBytes(intBytes)
176+
177+
var positive uint8
178+
if !s.ReadUint8(&positive) {
201179
return fmt.Errorf("data not long enough")
202180
}
203-
if data[where] == 1 {
204-
w.Positive = true
205-
} else {
206-
w.Positive = false
207-
}
181+
w.Positive = positive == 1
208182
return nil
209183
}
210184

@@ -213,75 +187,57 @@ func (w *Wire) Equal(w2 *Wire) bool {
213187
}
214188

215189
func (p *Policy) MarshalBinary() ([]byte, error) {
216-
ret := make([]byte, 2)
217190
fBytes, err := p.F.MarshalBinary()
218191
if err != nil {
219192
return nil, err
220193
}
221-
if len(fBytes) > math.MaxUint16 {
222-
return nil, fmt.Errorf("data too long")
223-
}
224-
binary.LittleEndian.PutUint16(ret[0:2], uint16(len(fBytes)))
225-
ret = append(ret, fBytes...)
226-
if len(p.Inputs) > math.MaxUint16 {
194+
b := cryptobyte.NewBuilder(nil)
195+
b.AddValue(lenPrefixed16(fBytes))
196+
if err = leUint16(len(p.Inputs)).Marshal(b); err != nil {
227197
return nil, fmt.Errorf("too many wires")
228198
}
229-
ret = append(ret, 0, 0)
230-
binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(p.Inputs)))
231199
for i := 0; i < len(p.Inputs); i++ {
232200
input, err := p.Inputs[i].MarshalBinary()
233201
if err != nil {
234202
return nil, err
235203
}
236-
if len(input) > math.MaxUint16 {
237-
return nil, fmt.Errorf("data too long")
238-
}
239-
ret = append(ret, 0, 0)
240-
binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(input)))
241-
ret = append(ret, input...)
204+
b.AddValue(lenPrefixed16(input))
242205
}
243-
return ret, nil
206+
return b.Bytes()
244207
}
245208

246209
func (p *Policy) UnmarshalBinary(data []byte) error {
210+
s := cryptobyte.String(data)
211+
247212
// Extract formula
248-
if len(data) < 2 {
249-
return fmt.Errorf("data not long enough")
250-
}
251-
fLen := uint(binary.LittleEndian.Uint16(data))
252-
data = data[2:]
253-
if uint(len(data)) < fLen {
213+
fLen, ok := readLEUint16(&s)
214+
var fBytes []byte
215+
if !ok || !s.ReadBytes(&fBytes, int(fLen)) {
254216
return fmt.Errorf("data not long enough")
255217
}
256-
err := p.F.UnmarshalBinary(data[:fLen])
257-
if err != nil {
218+
if err := p.F.UnmarshalBinary(fBytes); err != nil {
258219
return err
259220
}
260-
data = data[fLen:]
221+
261222
// Extract wires
262-
if len(data) < 2 {
223+
nWires16, ok := readLEUint16(&s)
224+
if !ok {
263225
return fmt.Errorf("data not long enough")
264226
}
265-
nWires := int(binary.LittleEndian.Uint16(data))
227+
nWires := int(nWires16)
266228
if nWires != len(p.F.Gates)+1 {
267229
return fmt.Errorf("invalid policy: %d wires declared, but a formula with %d gates requires exactly %d input wires", nWires, len(p.F.Gates), len(p.F.Gates)+1)
268230
}
269-
data = data[2:]
270231
p.Inputs = make([]Wire, nWires)
271-
for i := 0; i < nWires; i++ {
272-
if len(data) < 2 {
232+
for i := range nWires {
233+
wireLen, ok := readLEUint16(&s)
234+
var wireBytes []byte
235+
if !ok || !s.ReadBytes(&wireBytes, int(wireLen)) {
273236
return fmt.Errorf("data not long enough")
274237
}
275-
wireLen := uint(binary.LittleEndian.Uint16(data))
276-
data = data[2:]
277-
if uint(len(data)) < wireLen {
278-
return fmt.Errorf("data not long enough")
279-
}
280-
err = p.Inputs[i].UnmarshalBinary(data[:wireLen])
281-
if err != nil {
238+
if err := p.Inputs[i].UnmarshalBinary(wireBytes); err != nil {
282239
return fmt.Errorf("data not long enough")
283240
}
284-
data = data[wireLen:]
285241
}
286242
return nil
287243
}

0 commit comments

Comments
 (0)