RIN-02 Panic Due to Integer Overflow in Signature Size Calculation (#401

) * RIN-02 Panic Due to Integer Overflow in Signature Size Calculation * assert the error * simply overflow check * simply overflow check --------- Co-authored-by: Liam Lai <[email protected]>
XinFinOrg · Feb 3, 2024 · fea90a9 · fea90a9
1 parent 95e108c
commit fea90a9
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 1 deletion.
diff --git a/core/vm/privacy/ringct.go b/core/vm/privacy/ringct.go
@@ -176,6 +176,20 @@ func (r *RingSignature) Serialize() ([]byte, error) {
 }
 
 func computeSignatureSize(numRing int, ringSize int) int {
+	const MaxInt = int(^uint(0) >> 1)
+
+	if numRing < 0 || ringSize < 0 {
+		return -1
+	}
+
+	// Calculate term and check for overflow
+
+	term := numRing * ringSize * 65
+
+	if term < 0 || term < numRing || term < ringSize {
+		return -1
+	}
+
 	return 8 + 8 + 32 + 32 + numRing*ringSize*32 + numRing*ringSize*33 + numRing*33
 }
 
@@ -200,7 +214,7 @@ func Deserialize(r []byte) (*RingSignature, error) {
 	sig.NumRing = size_int
 
 	if len(r) != computeSignatureSize(sig.NumRing, sig.Size) {
-		return nil, errors.New("incorrect ring size")
+		return nil, fmt.Errorf("incorrect ring size, len r: %d, sig.NumRing: %d sig.Size: %d", len(r), sig.NumRing, sig.Size)
 	}
 
 	m := r[offset : offset+32]

diff --git a/core/vm/privacy/ringct_test.go b/core/vm/privacy/ringct_test.go
@@ -2,6 +2,7 @@ package privacy
 
 import (
 	"bytes"
+	"encoding/binary"
 	"fmt"
 	"testing"
 
@@ -48,6 +49,40 @@ func TestSign(t *testing.T) {
 
 }
 
+func TestDeserialize(t *testing.T) {
+	numRing := 5
+	ringSize := 10
+	s := 5
+	rings, privkeys, m, err := GenerateMultiRingParams(numRing, ringSize, s)
+
+	ringSignature, err := Sign(m, rings, privkeys, s)
+	if err != nil {
+		t.Error("Failed to create Ring signature")
+	}
+
+	// A normal signature.
+	sig, err := ringSignature.Serialize()
+	if err != nil {
+		t.Error("Failed to Serialize input Ring signature")
+	}
+
+	// Modify the serialized signature s.t.
+	// the new signature passes the length check
+	// but triggers buffer overflow in Deserialize().
+	// ringSize: 10 -> 56759212534490939
+	// len(sig): 3495 -> 3804
+	// 80 + 5 * (56759212534490939*65 + 33) = 18446744073709551616 + 3804
+	bs := make([]byte, 8)
+	binary.BigEndian.PutUint64(bs, 56759212534490939)
+	for i := 0; i < 8; i++ {
+		sig[i+8] = bs[i]
+	}
+	tail := make([]byte, 3804-len(sig))
+	sig = append(sig, tail...)
+
+	_, err = Deserialize(sig)
+	assert.EqualError(t, err, "incorrect ring size, len r: 3804, sig.NumRing: 5 sig.Size: 56759212534490939")
+}
 func TestPadTo32Bytes(t *testing.T) {
 	arr := [44]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34}