From 9f3f94fd5f9efd225cecc858bca6a9a2ccfbb57a Mon Sep 17 00:00:00 2001 From: Eamon Caton Date: Wed, 26 Aug 2026 20:30:10 +0900 Subject: [PATCH 1/2] Fix large policies resulting in undecryptable ciphertexts Because of the use of uint16, >65535 byte values put into the header will wrap around silently and cause decryption to later fail with a confusing "too short" message. This fixes encryption so that it rejects loudly and this can't silently occur (either accidentally or by malicious policy) --- abe/cpabe/tkn20/internal/tkn/bk.go | 25 ++++++-- abe/cpabe/tkn20/internal/tkn/formula.go | 20 +++++-- abe/cpabe/tkn20/internal/tkn/policy.go | 16 +++++ abe/cpabe/tkn20/internal/tkn/tk.go | 78 ++++++++++++++++++++----- abe/cpabe/tkn20/internal/tkn/util.go | 30 +++++++--- abe/cpabe/tkn20/tkn20_test.go | 71 ++++++++++++++++++++++ 6 files changed, 210 insertions(+), 30 deletions(-) diff --git a/abe/cpabe/tkn20/internal/tkn/bk.go b/abe/cpabe/tkn20/internal/tkn/bk.go index 3fabd681..9018f6d8 100644 --- a/abe/cpabe/tkn20/internal/tkn/bk.go +++ b/abe/cpabe/tkn20/internal/tkn/bk.go @@ -121,8 +121,14 @@ func EncryptCCA(rand io.Reader, public *PublicParams, policy *Policy, msg []byte if err != nil { return nil, err } - macData := appendLen32Prefixed(nil, C1) - macData = appendLen32Prefixed(macData, env) + macData, err := appendLen32Prefixed(nil, C1) + if err != nil { + return nil, err + } + macData, err = appendLen32Prefixed(macData, env) + if err != nil { + return nil, err + } tag, err := blakeMac(macKey, macData) if err != nil { @@ -130,9 +136,18 @@ func EncryptCCA(rand io.Reader, public *PublicParams, policy *Policy, msg []byte } ret := append([]byte{}, []byte(CiphertextVersion)...) - ret = appendLenPrefixed(ret, id) - ret = appendLen32Prefixed(ret, macData) - ret = appendLenPrefixed(ret, tag) + ret, err = appendLenPrefixed(ret, id) + if err != nil { + return nil, err + } + ret, err = appendLen32Prefixed(ret, macData) + if err != nil { + return nil, err + } + ret, err = appendLenPrefixed(ret, tag) + if err != nil { + return nil, err + } return ret, nil } diff --git a/abe/cpabe/tkn20/internal/tkn/formula.go b/abe/cpabe/tkn20/internal/tkn/formula.go index 39a9f755..03aa4e0e 100644 --- a/abe/cpabe/tkn20/internal/tkn/formula.go +++ b/abe/cpabe/tkn20/internal/tkn/formula.go @@ -4,6 +4,7 @@ import ( "encoding/binary" "fmt" "io" + "math" ) const ( @@ -60,13 +61,22 @@ func (g Gate) Equal(g2 Gate) bool { func (f *Formula) MarshalBinary() ([]byte, error) { n := len(f.Gates) + if n > math.MaxUint16 { + return nil, fmt.Errorf("too many gates") + } ret := make([]byte, 2+7*n) binary.LittleEndian.PutUint16(ret, uint16(len(f.Gates))) - for i := 0; i < n; i++ { - ret[7*i+2] = byte(f.Gates[i].Class) - binary.LittleEndian.PutUint16(ret[7*i+2+1:], uint16(f.Gates[i].In0)) - binary.LittleEndian.PutUint16(ret[7*i+2+3:], uint16(f.Gates[i].In1)) - binary.LittleEndian.PutUint16(ret[7*i+2+5:], uint16(f.Gates[i].Out)) + for i, gate := range f.Gates { + if gate.In0 < 0 || gate.In0 > math.MaxUint16 || + gate.In1 < 0 || gate.In1 > math.MaxUint16 || + gate.Out < 0 || gate.Out > math.MaxUint16 { + return nil, fmt.Errorf("wire index out of range") + } + + ret[7*i+2] = byte(gate.Class) + binary.LittleEndian.PutUint16(ret[7*i+2+1:], uint16(gate.In0)) + binary.LittleEndian.PutUint16(ret[7*i+2+3:], uint16(gate.In1)) + binary.LittleEndian.PutUint16(ret[7*i+2+5:], uint16(gate.Out)) } return ret, nil } diff --git a/abe/cpabe/tkn20/internal/tkn/policy.go b/abe/cpabe/tkn20/internal/tkn/policy.go index cb461330..9d3a6c26 100644 --- a/abe/cpabe/tkn20/internal/tkn/policy.go +++ b/abe/cpabe/tkn20/internal/tkn/policy.go @@ -3,6 +3,7 @@ package tkn import ( "encoding/binary" "fmt" + "math" pairing "github.com/cloudflare/circl/ecc/bls12381" ) @@ -74,6 +75,9 @@ func (a *Attribute) Equal(b *Attribute) bool { type Attributes map[string]Attribute func (a *Attributes) marshalBinary() ([]byte, error) { + if len(*a) > math.MaxUint16 { + return nil, fmt.Errorf("too many attributes (overflow)") + } ret := make([]byte, 2) binary.LittleEndian.PutUint16(ret[0:], uint16(len(*a))) @@ -135,6 +139,9 @@ func (w *Wire) MarshalBinary() ([]byte, error) { if err != nil { return nil, err } + if len(strBytes) > math.MaxUint16 || len(valBytes) > math.MaxUint16 || len(intBytes) > math.MaxUint16 { + return nil, fmt.Errorf("data too long") + } totalLen := len(strBytes) + len(valBytes) + len(intBytes) + 2 + 2 + 2 + 1 ret := make([]byte, totalLen) where := 0 @@ -211,8 +218,14 @@ func (p *Policy) MarshalBinary() ([]byte, error) { if err != nil { return nil, err } + if len(fBytes) > math.MaxUint16 { + return nil, fmt.Errorf("data too long") + } binary.LittleEndian.PutUint16(ret[0:2], uint16(len(fBytes))) ret = append(ret, fBytes...) + if len(p.Inputs) > math.MaxUint16 { + return nil, fmt.Errorf("too many wires") + } ret = append(ret, 0, 0) binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(p.Inputs))) for i := 0; i < len(p.Inputs); i++ { @@ -220,6 +233,9 @@ func (p *Policy) MarshalBinary() ([]byte, error) { if err != nil { return nil, err } + if len(input) > math.MaxUint16 { + return nil, fmt.Errorf("data too long") + } ret = append(ret, 0, 0) binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(input))) ret = append(ret, input...) diff --git a/abe/cpabe/tkn20/internal/tkn/tk.go b/abe/cpabe/tkn20/internal/tkn/tk.go index b33d7cc4..b310272a 100644 --- a/abe/cpabe/tkn20/internal/tkn/tk.go +++ b/abe/cpabe/tkn20/internal/tkn/tk.go @@ -7,6 +7,7 @@ import ( "encoding/binary" "fmt" "io" + "math" pairing "github.com/cloudflare/circl/ecc/bls12381" ) @@ -22,19 +23,28 @@ func (p *PublicParams) MarshalBinary() ([]byte, error) { if err != nil { return nil, fmt.Errorf("PublicParams serializing failed: %w", err) } - ret := appendLenPrefixed(nil, b2Bytes) + ret, err := appendLenPrefixed(nil, b2Bytes) + if err != nil { + return nil, fmt.Errorf("PublicParams serializing failed: %w", err) + } wb1Bytes, err := p.wb1.marshalBinary() if err != nil { return nil, fmt.Errorf("PublicParams serializing failed: %w", err) } - ret = appendLenPrefixed(ret, wb1Bytes) + ret, err = appendLenPrefixed(ret, wb1Bytes) + if err != nil { + return nil, fmt.Errorf("PublicParams serializing failed: %w", err) + } btkBytes, err := p.btk.marshalBinary() if err != nil { return nil, fmt.Errorf("PublicParams serializing failed: %w", err) } - ret = appendLenPrefixed(ret, btkBytes) + ret, err = appendLenPrefixed(ret, btkBytes) + if err != nil { + return nil, fmt.Errorf("PublicParams serializing failed: %w", err) + } return ret, nil } @@ -114,9 +124,15 @@ func (s *SecretParams) MarshalBinary() ([]byte, error) { aBytes, wtABytes, bstarBytes, bstar12Bytes, kBytes, s.prfKey, } - ret := appendLenPrefixed(nil, bufs[0]) + ret, err := appendLenPrefixed(nil, bufs[0]) + if err != nil { + return nil, fmt.Errorf("SecretParams serializing failed: %w", err) + } for _, buf := range bufs[1:] { - ret = appendLenPrefixed(ret, buf) + ret, err = appendLenPrefixed(ret, buf) + if err != nil { + return nil, fmt.Errorf("SecretParams serializing failed: %w", err) + } } return ret, nil } @@ -198,17 +214,29 @@ func (a *AttributesKey) MarshalBinary() ([]byte, error) { if err != nil { return nil, fmt.Errorf("AttributesKey serializing failed: %w", err) } - ret := appendLenPrefixed(nil, aBytes) + ret, err := appendLenPrefixed(nil, aBytes) + if err != nil { + return nil, fmt.Errorf("AttributesKey serializing failed: %w", err) + } k1Bytes, err := a.k1.marshalBinary() if err != nil { return nil, fmt.Errorf("AttributesKey serializing failed: %w", err) } - ret = appendLenPrefixed(ret, k1Bytes) + ret, err = appendLenPrefixed(ret, k1Bytes) + if err != nil { + return nil, fmt.Errorf("AttributesKey serializing failed: %w", err) + } k2Bytes, err := a.k2.marshalBinary() if err != nil { return nil, fmt.Errorf("AttributesKey serializing failed: %w", err) } - ret = appendLenPrefixed(ret, k2Bytes) + ret, err = appendLenPrefixed(ret, k2Bytes) + if err != nil { + return nil, fmt.Errorf("AttributesKey serializing failed: %w", err) + } + if len(a.k3) > math.MaxUint16 { + return nil, fmt.Errorf("AttributesKey serializing failed: too large (overflow)") + } ret = append(ret, 0, 0) binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(a.k3))) k3Bytes, err := marshalBinarySortedMapMatrixG1(a.k3) @@ -217,6 +245,9 @@ func (a *AttributesKey) MarshalBinary() ([]byte, error) { } ret = append(ret, k3Bytes...) + if len(a.k3wild) > math.MaxUint16 { + return nil, fmt.Errorf("AttributesKey serializing failed: too large (overflow)") + } ret = append(ret, 0, 0) binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(a.k3wild))) k3wildBytes, err := marshalBinarySortedMapMatrixG1(a.k3wild) @@ -343,19 +374,28 @@ func (hdr *ciphertextHeader) marshalBinary() ([]byte, error) { if err != nil { return nil, err } - ret := appendLenPrefixed(nil, pBytes) + ret, err := appendLenPrefixed(nil, pBytes) + if err != nil { + return nil, err + } c1Bytes, err := hdr.c1.marshalBinary() if err != nil { return nil, fmt.Errorf("c1 serializing: %w", err) } - ret = appendLenPrefixed(ret, c1Bytes) + ret, err = appendLenPrefixed(ret, c1Bytes) + if err != nil { + return nil, err + } // Now we need to indicate how long c2, c3, c3neg are. // Each array will be the same size (or nil), so with more work we can specialize // but for now we will ignore that. c2Len := len(hdr.c2) + if c2Len > math.MaxUint16 { + return nil, fmt.Errorf("c2 too long (overflow)") + } ret = append(ret, 0, 0) binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(c2Len)) @@ -364,9 +404,15 @@ func (hdr *ciphertextHeader) marshalBinary() ([]byte, error) { if errM != nil { return nil, fmt.Errorf("c2 serializing %d: %w", i, errM) } - ret = appendLenPrefixed(ret, c2dat) + ret, err = appendLenPrefixed(ret, c2dat) + if err != nil { + return nil, fmt.Errorf("c2 serializing %d: %w", i, err) + } } c3Len := len(hdr.c3) + if c3Len > math.MaxUint16 { + return nil, fmt.Errorf("c3 too long (overflow)") + } ret = append(ret, 0, 0) binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(c3Len)) for i := 0; i < c3Len; i++ { @@ -374,7 +420,10 @@ func (hdr *ciphertextHeader) marshalBinary() ([]byte, error) { if errM != nil { return nil, fmt.Errorf("c3 serializing %d: %w", i, errM) } - ret = appendLenPrefixed(ret, c3dat) + ret, err = appendLenPrefixed(ret, c3dat) + if err != nil { + return nil, fmt.Errorf("c3 seriaizing %d: %w", i, err) + } } for i := 0; i < c3Len; i++ { var c3negdat []byte @@ -386,7 +435,10 @@ func (hdr *ciphertextHeader) marshalBinary() ([]byte, error) { } else { c3negdat = nil } - ret = appendLenPrefixed(ret, c3negdat) + ret, err = appendLenPrefixed(ret, c3negdat) + if err != nil { + return nil, fmt.Errorf("c3neg serializing %d: %w", i, err) + } } return ret, nil } diff --git a/abe/cpabe/tkn20/internal/tkn/util.go b/abe/cpabe/tkn20/internal/tkn/util.go index 6aac952f..3059f935 100644 --- a/abe/cpabe/tkn20/internal/tkn/util.go +++ b/abe/cpabe/tkn20/internal/tkn/util.go @@ -4,6 +4,7 @@ import ( "encoding/binary" "errors" "fmt" + "math" "sort" pairing "github.com/cloudflare/circl/ecc/bls12381" @@ -42,11 +43,14 @@ func HashStringToScalar(key []byte, value string) *pairing.Scalar { return s } -func appendLen16Prefixed(a []byte, b []byte) []byte { +func appendLen16Prefixed(a []byte, b []byte) ([]byte, error) { + if len(b) > math.MaxUint16 { + return nil, fmt.Errorf("data too large (overflow)") + } a = append(a, 0, 0) binary.LittleEndian.PutUint16(a[len(a)-2:], uint16(len(b))) a = append(a, b...) - return a + return a, nil } func removeLen16Prefixed(data []byte) (next []byte, remainder []byte, err error) { @@ -65,11 +69,14 @@ var ( removeLenPrefixed = removeLen16Prefixed ) -func appendLen32Prefixed(a []byte, b []byte) []byte { +func appendLen32Prefixed(a []byte, b []byte) ([]byte, error) { + if len(b) > math.MaxUint32 { + return nil, fmt.Errorf("data too large (overflow)") + } a = append(a, 0, 0, 0, 0) binary.LittleEndian.PutUint32(a[len(a)-4:], uint32(len(b))) a = append(a, b...) - return a + return a, nil } func removeLen32Prefixed(data []byte) (next []byte, remainder []byte, err error) { @@ -100,8 +107,14 @@ func marshalBinarySortedMapMatrixG1(m map[string]*matrixG1) ([]byte, error) { return nil, err } - ret = appendLenPrefixed(ret, []byte(key)) - ret = appendLenPrefixed(ret, b) + ret, err = appendLenPrefixed(ret, []byte(key)) + if err != nil { + return nil, err + } + ret, err = appendLenPrefixed(ret, b) + if err != nil { + return nil, err + } } return ret, nil @@ -122,7 +135,10 @@ func marshalBinarySortedMapAttribute(m map[string]Attribute) ([]byte, error) { return nil, err } - ret = appendLenPrefixed(ret, []byte(key)) + ret, err = appendLenPrefixed(ret, []byte(key)) + if err != nil { + return nil, err + } ret = append(ret, b...) } diff --git a/abe/cpabe/tkn20/tkn20_test.go b/abe/cpabe/tkn20/tkn20_test.go index 1a6ca63b..7fe6eae9 100644 --- a/abe/cpabe/tkn20/tkn20_test.go +++ b/abe/cpabe/tkn20/tkn20_test.go @@ -527,3 +527,74 @@ func TestPolicyExtractMutatedCiphertextStackOverflow(t *testing.T) { //_ = extracted.String() // fatal error: stack overflow — kills the entire process //t.Fatal("unreachable: String() should have crashed the process") } + +// Encrypt was allowed to build a large enough ciphertext such that it cannot be decrypted later +// +// ciphertextHeader.marshalBinary stores the length in a uint16 field, so any value above 65535 +// bytes wraps around. Decryption (obviously) fails on this, but it also cannot recover the data, +// and gives a confusing 'too short' error message +// +// Note the issue is the overall length, not purely number of attributes or purely their lengths +func TestEncryptWrapsAroundWithLongValues(t *testing.T) { + const label = "contributor_email_address" + const n = 700 // ~630 or so is necessary to trigger this + const idx = n / 2 + + adjectives := []string{"billowy", "righteous", "abstracted", "statuesque", "furry", "responsible", "cluttered", "quietly", "calmly", "strange"} + descriptors := []string{"pupil", "newsletter", "puppy", "stormy", "stupendous", "sugar", "supportive", "rebirth", "pacific", "reform"} + nouns := []string{"reaction", "rabbit", "argument", "reading", "song", "heat", "books", "wash", "trip", "instrument"} + + value := func(i int) string { + return fmt.Sprintf("%08d_%s_%s_%s", 10_000_000+i, adjectives[i%len(adjectives)], descriptors[(i/3)%len(descriptors)], nouns[(i/7)%len(nouns)]) + } + + clauses := make([]string, n) + var val string + for i := range n { + v := value(i) + clauses[i] = fmt.Sprintf("%s:%s", label, v) + if i == idx { + // Pick a 'random' value we know must suffice + val = v + } + } + // OR is chosen for speed/simplicity; there is nothing particular about it that causes the + // issue + policyStr := strings.Join(clauses, " or ") + + var policy Policy + if err := policy.FromString(policyStr); err != nil { + t.Fatalf("Failed to parse clause policy: %v", err) + } + + pk, msk, err := Setup(rand.Reader) + if err != nil { + t.Fatalf("Failed to complete setup: %v", err) + } + + msg := []byte("CIRCL (Cloudflare Interoperable, Reusable Cryptographic Library) is a collection of cryptographic primitives written in Go.") + ct, err := pk.Encrypt(rand.Reader, policy, msg) + if err != nil { + // Expected, return + return + } + + // otherwise we should assert the following on fail + var attrs Attributes + attrs.FromMap(map[string]string{label: val}) + key, err := msk.KeyGen(rand.Reader, attrs) + if err != nil { + t.Fatalf("Key generation failed: %v", err) + } + + pt, err := key.Decrypt(ct) + if err != nil { + t.Fatalf("Failed to decrypt despite knowing %q satisfies: %v", val, err) + } + // Sanity check + if string(pt) != string(msg) { + t.Fatalf("Decryption succeeded, but %q != %q", pt, msg) + } + // Failsafe + t.Fatalf("Failed to encrypt message: %v", err) +} From d31e261a16699d346edaa1e5cd531736a714ba05 Mon Sep 17 00:00:00 2001 From: Eamon Caton Date: Tue, 22 Sep 2026 18:28:21 +0900 Subject: [PATCH 2/2] 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 --- abe/cpabe/tkn20/internal/tkn/formula.go | 58 +++++---- abe/cpabe/tkn20/internal/tkn/policy.go | 160 +++++++++--------------- abe/cpabe/tkn20/internal/tkn/tk.go | 104 ++++++++------- abe/cpabe/tkn20/internal/tkn/util.go | 114 +++++++++++++---- abe/cpabe/tkn20/tkn20_test.go | 2 - 5 files changed, 235 insertions(+), 203 deletions(-) diff --git a/abe/cpabe/tkn20/internal/tkn/formula.go b/abe/cpabe/tkn20/internal/tkn/formula.go index 03aa4e0e..9c93e334 100644 --- a/abe/cpabe/tkn20/internal/tkn/formula.go +++ b/abe/cpabe/tkn20/internal/tkn/formula.go @@ -1,10 +1,10 @@ package tkn import ( - "encoding/binary" "fmt" "io" - "math" + + "golang.org/x/crypto/cryptobyte" ) const ( @@ -61,40 +61,44 @@ func (g Gate) Equal(g2 Gate) bool { func (f *Formula) MarshalBinary() ([]byte, error) { n := len(f.Gates) - if n > math.MaxUint16 { - return nil, fmt.Errorf("too many gates") + b := cryptobyte.NewBuilder(nil) + b.AddValue(leUint16(n)) + for _, gate := range f.Gates { + b.AddUint8(byte(gate.Class)) + b.AddValue(leUint16(gate.In0)) + b.AddValue(leUint16(gate.In1)) + b.AddValue(leUint16(gate.Out)) } - ret := make([]byte, 2+7*n) - binary.LittleEndian.PutUint16(ret, uint16(len(f.Gates))) - for i, gate := range f.Gates { - if gate.In0 < 0 || gate.In0 > math.MaxUint16 || - gate.In1 < 0 || gate.In1 > math.MaxUint16 || - gate.Out < 0 || gate.Out > math.MaxUint16 { - return nil, fmt.Errorf("wire index out of range") - } - - ret[7*i+2] = byte(gate.Class) - binary.LittleEndian.PutUint16(ret[7*i+2+1:], uint16(gate.In0)) - binary.LittleEndian.PutUint16(ret[7*i+2+3:], uint16(gate.In1)) - binary.LittleEndian.PutUint16(ret[7*i+2+5:], uint16(gate.Out)) + ret, err := b.Bytes() + if err != nil { + return nil, err } return ret, nil } func (f *Formula) UnmarshalBinary(data []byte) error { - if len(data) < 2 { - return fmt.Errorf("too short data") - } - n := int(binary.LittleEndian.Uint16(data[0:2])) - if len(data) < 2+7*n { + s := cryptobyte.String(data) + n16, ok := readLEUint16(&s) + if !ok { return fmt.Errorf("too short data") } + n := int(n16) f.Gates = make([]Gate, n) - for i := 0; i < n; i++ { - f.Gates[i].Class = int(data[7*i+2]) - f.Gates[i].In0 = int(binary.LittleEndian.Uint16(data[7*i+2+1:])) - f.Gates[i].In1 = int(binary.LittleEndian.Uint16(data[7*i+2+3:])) - f.Gates[i].Out = int(binary.LittleEndian.Uint16(data[7*i+2+5:])) + for i := range n { + var class uint8 + if !s.ReadUint8(&class) { + return fmt.Errorf("too short data") + } + in0, ok0 := readLEUint16(&s) + in1, ok1 := readLEUint16(&s) + out, ok2 := readLEUint16(&s) + if !ok0 || !ok1 || !ok2 { + return fmt.Errorf("too short data") + } + f.Gates[i].Class = int(class) + f.Gates[i].In0 = int(in0) + f.Gates[i].In1 = int(in1) + f.Gates[i].Out = int(out) } // Reject malformed or cyclic gate graphs coming from untrusted encodings. check := Formula{Gates: append([]Gate(nil), f.Gates...)} diff --git a/abe/cpabe/tkn20/internal/tkn/policy.go b/abe/cpabe/tkn20/internal/tkn/policy.go index 9d3a6c26..18f2ca28 100644 --- a/abe/cpabe/tkn20/internal/tkn/policy.go +++ b/abe/cpabe/tkn20/internal/tkn/policy.go @@ -1,11 +1,10 @@ package tkn import ( - "encoding/binary" "fmt" - "math" pairing "github.com/cloudflare/circl/ecc/bls12381" + "golang.org/x/crypto/cryptobyte" ) const ( @@ -75,29 +74,28 @@ func (a *Attribute) Equal(b *Attribute) bool { type Attributes map[string]Attribute func (a *Attributes) marshalBinary() ([]byte, error) { - if len(*a) > math.MaxUint16 { - return nil, fmt.Errorf("too many attributes (overflow)") - } - ret := make([]byte, 2) - binary.LittleEndian.PutUint16(ret[0:], uint16(len(*a))) - aBytes, err := marshalBinarySortedMapAttribute(*a) if err != nil { return nil, fmt.Errorf("marshalling Attributes failed: %w", err) } - ret = append(ret, aBytes...) - - return ret, nil + b := cryptobyte.NewBuilder(nil) + if err = leUint16(len(*a)).Marshal(b); err != nil { + return nil, fmt.Errorf("too many attributes") + } + b.AddBytes(aBytes) + return b.Bytes() } func (a *Attributes) unmarshalBinary(data []byte) error { - if len(data) < 2 { + s := cryptobyte.String(data) + n16, ok := readLEUint16(&s) + if !ok { return fmt.Errorf("unmarshalling Attributes failed: data too short") } - n := int(binary.LittleEndian.Uint16(data)) - data = data[2:] + n := int(n16) + data = s *a = make(map[string]Attribute, n) - for i := 0; i < n; i++ { + for range n { labelBytes, rem, err := removeLenPrefixed(data) if err != nil { return fmt.Errorf("unmarshalling Attributes failed: %w", err) @@ -139,72 +137,48 @@ func (w *Wire) MarshalBinary() ([]byte, error) { if err != nil { return nil, err } - if len(strBytes) > math.MaxUint16 || len(valBytes) > math.MaxUint16 || len(intBytes) > math.MaxUint16 { - return nil, fmt.Errorf("data too long") - } - totalLen := len(strBytes) + len(valBytes) + len(intBytes) + 2 + 2 + 2 + 1 - ret := make([]byte, totalLen) - where := 0 - binary.LittleEndian.PutUint16(ret[where:], uint16(len(strBytes))) - where += 2 - where += copy(ret[where:], strBytes) - binary.LittleEndian.PutUint16(ret[where:], uint16(len(valBytes))) - where += 2 - where += copy(ret[where:], valBytes) - binary.LittleEndian.PutUint16(ret[where:], uint16(len(intBytes))) - where += 2 - where += copy(ret[where:], intBytes) + b := cryptobyte.NewBuilder(nil) + b.AddValue(lenPrefixed16(strBytes)) + b.AddValue(lenPrefixed16(valBytes)) + b.AddValue(lenPrefixed16(intBytes)) if w.Positive { - ret[where] = 1 + b.AddUint8(1) } else { - ret[where] = 0 + b.AddUint8(0) } - return ret, nil + return b.Bytes() } func (w *Wire) UnmarshalBinary(data []byte) error { - where := 0 - if len(data) < 2 { - return fmt.Errorf("data not long enough") - } - strLen := int(binary.LittleEndian.Uint16(data[where:])) - where += 2 - if len(data[where:]) < strLen { - return fmt.Errorf("data not long enough") - } - w.Label = string(data[where : where+strLen]) - where += strLen + s := cryptobyte.String(data) - if len(data[where:]) < 2 { - return fmt.Errorf("data not long enough") - } - valLen := int(binary.LittleEndian.Uint16(data[where:])) - where += 2 - if len(data[where:]) < valLen { + strLen, ok := readLEUint16(&s) + var strBytes []byte + if !ok || !s.ReadBytes(&strBytes, int(strLen)) { return fmt.Errorf("data not long enough") } - w.RawValue = string(data[where : where+valLen]) - where += valLen + w.Label = string(strBytes) - if len(data[where:]) < 2 { + valLen, ok := readLEUint16(&s) + var valBytes []byte + if !ok || !s.ReadBytes(&valBytes, int(valLen)) { return fmt.Errorf("data not long enough") } - intLen := int(binary.LittleEndian.Uint16(data[where:])) - where += 2 - if len(data[where:]) < intLen { + w.RawValue = string(valBytes) + + intLen, ok := readLEUint16(&s) + var intBytes []byte + if !ok || !s.ReadBytes(&intBytes, int(intLen)) { return fmt.Errorf("data not long enough") } w.Value = &pairing.Scalar{} - w.Value.SetBytes(data[where : where+intLen]) - where += intLen - if len(data[where:]) < 1 { + w.Value.SetBytes(intBytes) + + var positive uint8 + if !s.ReadUint8(&positive) { return fmt.Errorf("data not long enough") } - if data[where] == 1 { - w.Positive = true - } else { - w.Positive = false - } + w.Positive = positive == 1 return nil } @@ -213,75 +187,57 @@ func (w *Wire) Equal(w2 *Wire) bool { } func (p *Policy) MarshalBinary() ([]byte, error) { - ret := make([]byte, 2) fBytes, err := p.F.MarshalBinary() if err != nil { return nil, err } - if len(fBytes) > math.MaxUint16 { - return nil, fmt.Errorf("data too long") - } - binary.LittleEndian.PutUint16(ret[0:2], uint16(len(fBytes))) - ret = append(ret, fBytes...) - if len(p.Inputs) > math.MaxUint16 { + b := cryptobyte.NewBuilder(nil) + b.AddValue(lenPrefixed16(fBytes)) + if err = leUint16(len(p.Inputs)).Marshal(b); err != nil { return nil, fmt.Errorf("too many wires") } - ret = append(ret, 0, 0) - binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(p.Inputs))) for i := 0; i < len(p.Inputs); i++ { input, err := p.Inputs[i].MarshalBinary() if err != nil { return nil, err } - if len(input) > math.MaxUint16 { - return nil, fmt.Errorf("data too long") - } - ret = append(ret, 0, 0) - binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(input))) - ret = append(ret, input...) + b.AddValue(lenPrefixed16(input)) } - return ret, nil + return b.Bytes() } func (p *Policy) UnmarshalBinary(data []byte) error { + s := cryptobyte.String(data) + // Extract formula - if len(data) < 2 { - return fmt.Errorf("data not long enough") - } - fLen := uint(binary.LittleEndian.Uint16(data)) - data = data[2:] - if uint(len(data)) < fLen { + fLen, ok := readLEUint16(&s) + var fBytes []byte + if !ok || !s.ReadBytes(&fBytes, int(fLen)) { return fmt.Errorf("data not long enough") } - err := p.F.UnmarshalBinary(data[:fLen]) - if err != nil { + if err := p.F.UnmarshalBinary(fBytes); err != nil { return err } - data = data[fLen:] + // Extract wires - if len(data) < 2 { + nWires16, ok := readLEUint16(&s) + if !ok { return fmt.Errorf("data not long enough") } - nWires := int(binary.LittleEndian.Uint16(data)) + nWires := int(nWires16) if nWires != len(p.F.Gates)+1 { 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) } - data = data[2:] p.Inputs = make([]Wire, nWires) - for i := 0; i < nWires; i++ { - if len(data) < 2 { + for i := range nWires { + wireLen, ok := readLEUint16(&s) + var wireBytes []byte + if !ok || !s.ReadBytes(&wireBytes, int(wireLen)) { return fmt.Errorf("data not long enough") } - wireLen := uint(binary.LittleEndian.Uint16(data)) - data = data[2:] - if uint(len(data)) < wireLen { - return fmt.Errorf("data not long enough") - } - err = p.Inputs[i].UnmarshalBinary(data[:wireLen]) - if err != nil { + if err := p.Inputs[i].UnmarshalBinary(wireBytes); err != nil { return fmt.Errorf("data not long enough") } - data = data[wireLen:] } return nil } diff --git a/abe/cpabe/tkn20/internal/tkn/tk.go b/abe/cpabe/tkn20/internal/tkn/tk.go index b310272a..375bd924 100644 --- a/abe/cpabe/tkn20/internal/tkn/tk.go +++ b/abe/cpabe/tkn20/internal/tkn/tk.go @@ -4,12 +4,11 @@ package tkn import ( "crypto/subtle" - "encoding/binary" "fmt" "io" - "math" pairing "github.com/cloudflare/circl/ecc/bls12381" + "golang.org/x/crypto/cryptobyte" ) type PublicParams struct { @@ -234,29 +233,27 @@ func (a *AttributesKey) MarshalBinary() ([]byte, error) { if err != nil { return nil, fmt.Errorf("AttributesKey serializing failed: %w", err) } - if len(a.k3) > math.MaxUint16 { - return nil, fmt.Errorf("AttributesKey serializing failed: too large (overflow)") - } - ret = append(ret, 0, 0) - binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(a.k3))) + k3Bytes, err := marshalBinarySortedMapMatrixG1(a.k3) if err != nil { return nil, fmt.Errorf("AttributesKey serializing failed: %w", err) } - ret = append(ret, k3Bytes...) - - if len(a.k3wild) > math.MaxUint16 { - return nil, fmt.Errorf("AttributesKey serializing failed: too large (overflow)") - } - ret = append(ret, 0, 0) - binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(len(a.k3wild))) k3wildBytes, err := marshalBinarySortedMapMatrixG1(a.k3wild) if err != nil { return nil, fmt.Errorf("AttributesKey serializing failed: %w", err) } - ret = append(ret, k3wildBytes...) - return ret, nil + b := cryptobyte.NewBuilder(ret) + if err = leUint16(len(a.k3)).Marshal(b); err != nil { + return nil, fmt.Errorf("AttributesKey serializing failed: too many entries") + } + b.AddBytes(k3Bytes) + if err = leUint16(len(a.k3wild)).Marshal(b); err != nil { + return nil, fmt.Errorf("AttributesKey serializing failed: too many entries") + } + b.AddBytes(k3wildBytes) + + return b.Bytes() } func (a *AttributesKey) UnmarshalBinary(data []byte) error { @@ -288,11 +285,13 @@ func (a *AttributesKey) UnmarshalBinary(data []byte) error { return fmt.Errorf("AttributesKey deserialization failure: %w", err) } - if len(data) < 2 { + s := cryptobyte.String(data) + n16, ok := readLEUint16(&s) + if !ok { return fmt.Errorf("AttributesKey deserialization failure: data too short") } - n := int(binary.LittleEndian.Uint16(data)) - data = data[2:] + n := int(n16) + data = s a.k3 = make(map[string]*matrixG1, n) for i := 0; i < n; i++ { sBytes, rem, err := removeLenPrefixed(data) @@ -312,11 +311,13 @@ func (a *AttributesKey) UnmarshalBinary(data []byte) error { data = rem } - if len(data) < 2 { + s = cryptobyte.String(data) + n16, ok = readLEUint16(&s) + if !ok { return fmt.Errorf("AttributesKey deserialization failure: data too short") } - n = int(binary.LittleEndian.Uint16(data)) - data = data[2:] + n = int(n16) + data = s a.k3wild = make(map[string]*matrixG1, n) for i := 0; i < n; i++ { sBytes, rem, err := removeLenPrefixed(data) @@ -376,7 +377,7 @@ func (hdr *ciphertextHeader) marshalBinary() ([]byte, error) { } ret, err := appendLenPrefixed(nil, pBytes) if err != nil { - return nil, err + return nil, fmt.Errorf("policy serializing: %w", err) } c1Bytes, err := hdr.c1.marshalBinary() @@ -385,7 +386,7 @@ func (hdr *ciphertextHeader) marshalBinary() ([]byte, error) { } ret, err = appendLenPrefixed(ret, c1Bytes) if err != nil { - return nil, err + return nil, fmt.Errorf("c1 serializing: %w", err) } // Now we need to indicate how long c2, c3, c3neg are. @@ -393,13 +394,15 @@ func (hdr *ciphertextHeader) marshalBinary() ([]byte, error) { // but for now we will ignore that. c2Len := len(hdr.c2) - if c2Len > math.MaxUint16 { - return nil, fmt.Errorf("c2 too long (overflow)") + b := cryptobyte.NewBuilder(ret) + if err = leUint16(c2Len).Marshal(b); err != nil { + return nil, fmt.Errorf("c2 too long") } - - ret = append(ret, 0, 0) - binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(c2Len)) - for i := 0; i < c2Len; i++ { + ret, err = b.Bytes() + if err != nil { + return nil, fmt.Errorf("c2 too long") + } + for i := range c2Len { c2dat, errM := hdr.c2[i].marshalBinary() if errM != nil { return nil, fmt.Errorf("c2 serializing %d: %w", i, errM) @@ -410,22 +413,25 @@ func (hdr *ciphertextHeader) marshalBinary() ([]byte, error) { } } c3Len := len(hdr.c3) - if c3Len > math.MaxUint16 { - return nil, fmt.Errorf("c3 too long (overflow)") + b = cryptobyte.NewBuilder(ret) + if err = leUint16(c3Len).Marshal(b); err != nil { + return nil, fmt.Errorf("c3 too long") + } + ret, err = b.Bytes() + if err != nil { + return nil, fmt.Errorf("c3 too long") } - ret = append(ret, 0, 0) - binary.LittleEndian.PutUint16(ret[len(ret)-2:], uint16(c3Len)) - for i := 0; i < c3Len; i++ { + for i := range c3Len { c3dat, errM := hdr.c3[i].marshalBinary() if errM != nil { return nil, fmt.Errorf("c3 serializing %d: %w", i, errM) } ret, err = appendLenPrefixed(ret, c3dat) if err != nil { - return nil, fmt.Errorf("c3 seriaizing %d: %w", i, err) + return nil, fmt.Errorf("c3 serializing %d: %w", i, err) } } - for i := 0; i < c3Len; i++ { + for i := range c3Len { var c3negdat []byte if hdr.c3neg[i] != nil { c3negdat, err = hdr.c3neg[i].marshalBinary() @@ -462,17 +468,19 @@ func (hdr *ciphertextHeader) unmarshalBinary(data []byte) error { if err != nil { return err } - if len(data) < 2 { + s := cryptobyte.String(data) + c2Len16, ok := readLEUint16(&s) + if !ok { return fmt.Errorf("ciphertext header too short") } - c2Len := int(binary.LittleEndian.Uint16(data)) + c2Len := int(c2Len16) hdr.c2 = make([]*matrixG2, c2Len) - data = data[2:] + data = s var c2data []byte var c3data []byte var c3negdata []byte - for i := 0; i < c2Len; i++ { + for i := range c2Len { c2data, data, err = removeLenPrefixed(data) if err != nil { return err @@ -484,15 +492,17 @@ func (hdr *ciphertextHeader) unmarshalBinary(data []byte) error { } } - if len(data) < 2 { + s = cryptobyte.String(data) + c3Len16, ok := readLEUint16(&s) + if !ok { return fmt.Errorf("ciphertext header too short") } - c3Len := int(binary.LittleEndian.Uint16(data)) + c3Len := int(c3Len16) hdr.c3 = make([]*matrixG1, c3Len) hdr.c3neg = make([]*matrixG1, c3Len) - data = data[2:] + data = s - for i := 0; i < c3Len; i++ { + for i := range c3Len { c3data, data, err = removeLenPrefixed(data) if err != nil { return err @@ -504,7 +514,7 @@ func (hdr *ciphertextHeader) unmarshalBinary(data []byte) error { } } - for i := 0; i < c3Len; i++ { + for i := range c3Len { c3negdata, data, err = removeLenPrefixed(data) if err != nil { return err @@ -839,7 +849,7 @@ func decapsulate(header *ciphertextHeader, key *AttributesKey) (*pairing.Gt, err keymat.scalarMult(y, key.k3[mt.label]) keymat.add(keymat, key.k3wild[mt.label]) } else { - y.Set((*(key.a))[mt.label].Value) + y.Set((*key.a)[mt.label].Value) keymat.set(key.k3[mt.label]) } diff := &pairing.Scalar{} diff --git a/abe/cpabe/tkn20/internal/tkn/util.go b/abe/cpabe/tkn20/internal/tkn/util.go index 3059f935..7d245a45 100644 --- a/abe/cpabe/tkn20/internal/tkn/util.go +++ b/abe/cpabe/tkn20/internal/tkn/util.go @@ -9,6 +9,7 @@ import ( pairing "github.com/cloudflare/circl/ecc/bls12381" "golang.org/x/crypto/blake2b" + "golang.org/x/crypto/cryptobyte" ) var gtBaseVal *pairing.Gt @@ -43,25 +44,93 @@ func HashStringToScalar(key []byte, value string) *pairing.Scalar { return s } -func appendLen16Prefixed(a []byte, b []byte) ([]byte, error) { - if len(b) > math.MaxUint16 { - return nil, fmt.Errorf("data too large (overflow)") +// The tkn20 on-the-wire format uses little-endian length prefixes, but +// cryptobyte works in big-endian +// +// leUint16/leUint32 use cryptobyte.Builder as a bounds-checker but write the +// little-endian "by hand" and centralize the over/underflow check +type leUint16 int + +func (v leUint16) Marshal(b *cryptobyte.Builder) error { + if v < 0 || v > math.MaxUint16 { + return fmt.Errorf("data too long") } - a = append(a, 0, 0) - binary.LittleEndian.PutUint16(a[len(a)-2:], uint16(len(b))) - a = append(a, b...) - return a, nil + var buf [2]byte + binary.LittleEndian.PutUint16(buf[:], uint16(v)) + b.AddBytes(buf[:]) + return nil } +type leUint32 int + +func (v leUint32) Marshal(b *cryptobyte.Builder) error { + if v < 0 || v > math.MaxUint32 { + return fmt.Errorf("data too long") + } + var buf [4]byte + binary.LittleEndian.PutUint32(buf[:], uint32(v)) + b.AddBytes(buf[:]) + return nil +} + +type lenPrefixed16 []byte + +func (v lenPrefixed16) Marshal(b *cryptobyte.Builder) error { + if err := leUint16(len(v)).Marshal(b); err != nil { + return err + } + b.AddBytes(v) + return nil +} + +type lenPrefixed32 []byte + +func (v lenPrefixed32) Marshal(b *cryptobyte.Builder) error { + if err := leUint32(len(v)).Marshal(b); err != nil { + return err + } + b.AddBytes(v) + return nil +} + +// readLEUint16 and readLEUint32 read a little-endian length field which is +// bounds-checked by cryptobyte.String +func readLEUint16(s *cryptobyte.String) (uint16, bool) { + var raw []byte + if !s.ReadBytes(&raw, 2) { + return 0, false + } + return binary.LittleEndian.Uint16(raw), true +} + +func readLEUint32(s *cryptobyte.String) (uint32, bool) { + var raw []byte + if !s.ReadBytes(&raw, 4) { + return 0, false + } + return binary.LittleEndian.Uint32(raw), true +} + +func appendLen16Prefixed(a []byte, b []byte) ([]byte, error) { + bld := cryptobyte.NewBuilder(a) + bld.AddValue(lenPrefixed16(b)) + return bld.Bytes() +} + +// removeLen16Prefixed reads a little-endian uint16 length prefix from data +// via cryptobyte.String (ReadBytes rejects both short input and a negative +// length) func removeLen16Prefixed(data []byte) (next []byte, remainder []byte, err error) { - if len(data) < 2 { + s := cryptobyte.String(data) + n, ok := readLEUint16(&s) + if !ok { return nil, nil, fmt.Errorf("data too short") } - itemLen := int(binary.LittleEndian.Uint16(data)) - if (2 + itemLen) > len(data) { + var item []byte + if !s.ReadBytes(&item, int(n)) { return nil, nil, fmt.Errorf("data too short") } - return data[2 : 2+itemLen], data[2+itemLen:], nil + return item, s, nil } var ( @@ -70,27 +139,22 @@ var ( ) func appendLen32Prefixed(a []byte, b []byte) ([]byte, error) { - if len(b) > math.MaxUint32 { - return nil, fmt.Errorf("data too large (overflow)") - } - a = append(a, 0, 0, 0, 0) - binary.LittleEndian.PutUint32(a[len(a)-4:], uint32(len(b))) - a = append(a, b...) - return a, nil + bld := cryptobyte.NewBuilder(a) + bld.AddValue(lenPrefixed32(b)) + return bld.Bytes() } func removeLen32Prefixed(data []byte) (next []byte, remainder []byte, err error) { - if len(data) < 4 { + s := cryptobyte.String(data) + n, ok := readLEUint32(&s) + if !ok { return nil, nil, fmt.Errorf("data too short") } - itemLen := int(binary.LittleEndian.Uint32(data)) - if itemLen < 0 { - return nil, nil, fmt.Errorf("negative data length: possible overflow") - } - if (4 + itemLen) > len(data) { + var item []byte + if !s.ReadBytes(&item, int(n)) { return nil, nil, fmt.Errorf("data too short") } - return data[4 : 4+itemLen], data[4+itemLen:], nil + return item, s, nil } func marshalBinarySortedMapMatrixG1(m map[string]*matrixG1) ([]byte, error) { diff --git a/abe/cpabe/tkn20/tkn20_test.go b/abe/cpabe/tkn20/tkn20_test.go index 7fe6eae9..4bc1e564 100644 --- a/abe/cpabe/tkn20/tkn20_test.go +++ b/abe/cpabe/tkn20/tkn20_test.go @@ -595,6 +595,4 @@ func TestEncryptWrapsAroundWithLongValues(t *testing.T) { if string(pt) != string(msg) { t.Fatalf("Decryption succeeded, but %q != %q", pt, msg) } - // Failsafe - t.Fatalf("Failed to encrypt message: %v", err) }