Fix varint decoding bug

loopholelabs · Aug 26, 2023 · 3bdf786 · 3bdf786
1 parent a7d5107
commit 3bdf786
Show file tree

Hide file tree

Showing 6 changed files with 490 additions and 18 deletions.
diff --git a/decode.go b/decode.go
@@ -245,11 +245,13 @@ func decodeInt32(b []byte) ([]byte, int32, error) {
 				if i > VarIntLen32 && cb > 1 {
 					return b, 0, InvalidInt32
 				}
-				// End of varint, add the last bits and cast to signed integer
-				x := int32((ux | uint32(cb)<<s) >> 1)
-				// Flip the bits if the sign bit is set
+				// End of varint, add the last bits
+				ux |= uint32(cb) << s
+				// Separate value and sign
+				x := int32(ux >> 1)
+				// If sign bit is set, negate the number
 				if ux&1 != 0 {
-					x = ^x
+					x = -(x + 1)
 				}
 				return b[i+1:], x, nil
 			}
@@ -271,11 +273,13 @@ func decodeInt64(b []byte) ([]byte, int64, error) {
 				if i > VarIntLen64 && cb > 1 {
 					return b, 0, InvalidInt64
 				}
-				// End of varint, add the last bits and cast to signed integer
-				x := int64((ux | uint64(cb)<<s) >> 1)
-				// Flip the bits if the sign bit is set
+				// End of varint, add the last bits
+				ux |= uint64(cb) << s
+				// Separate value and sign
+				x := int64(ux >> 1)
+				// If sign bit is set, negate the number
 				if ux&1 != 0 {
-					x = ^x
+					x = -(x + 1)
 				}
 				return b[i+1:], x, nil
 			}

diff --git a/decode_test.go b/decode_test.go
@@ -456,7 +456,7 @@ func TestDecodeInt32(t *testing.T) {
 	t.Parallel()
 
 	p := NewBuffer()
-	v := int32(-2147483648)
+	v := int32(2147483647)
 	encodeInt32(p, v)
 
 	var value int32
@@ -466,6 +466,15 @@ func TestDecodeInt32(t *testing.T) {
 	assert.Equal(t, v, value)
 	assert.Equal(t, 0, len(remaining))
 
+	v = int32(-2147483647)
+	p.Reset()
+	encodeInt32(p, v)
+
+	remaining, value, err = decodeInt32(p.Bytes())
+	assert.NoError(t, err)
+	assert.Equal(t, v, value)
+	assert.Equal(t, 0, len(remaining))
+
 	_, _, err = decodeInt32((p.Bytes())[1:])
 	assert.ErrorIs(t, err, InvalidInt32)
 
@@ -491,7 +500,7 @@ func TestDecodeInt64(t *testing.T) {
 	t.Parallel()
 
 	p := NewBuffer()
-	v := int64(-9223372036854775808)
+	v := int64(9223372036854775807)
 	encodeInt64(p, v)
 
 	var value int64
@@ -501,6 +510,15 @@ func TestDecodeInt64(t *testing.T) {
 	assert.Equal(t, v, value)
 	assert.Equal(t, 0, len(remaining))
 
+	v = int64(-9223372036854775807)
+	p.Reset()
+	encodeInt64(p, v)
+
+	remaining, value, err = decodeInt64(p.Bytes())
+	assert.NoError(t, err)
+	assert.Equal(t, v, value)
+	assert.Equal(t, 0, len(remaining))
+
 	_, _, err = decodeInt64((p.Bytes())[1:])
 	assert.ErrorIs(t, err, InvalidInt64)
 

diff --git a/decoder.rs b/decoder.rs
@@ -254,9 +254,10 @@ impl Decoder for Cursor<&mut Vec<u8>> {
             for _ in 0..VARINT_LEN32 {
                 let byte = self.read_u8().ok().ok_or(DecodingError::InvalidI32)?;
                 if byte < CONTINUATION {
-                    let mut x = ((ux | (byte as u32) << s) >> 1) as i32;
+                    ux |= (byte as u32) << s;
+                    let mut x = (ux >> 1) as i32;
                     if ux & 1 != 0 {
-                        x = !x
+                        x = -(x + 1)
                     }
                     return Ok(x);
                 }
@@ -277,9 +278,10 @@ impl Decoder for Cursor<&mut Vec<u8>> {
             for _ in 0..VARINT_LEN64 {
                 let byte = self.read_u8().ok().ok_or(DecodingError::InvalidI64)?;
                 if byte < CONTINUATION {
-                    let mut x = ((ux | (byte as u64) << s) >> 1) as i64;
+                    ux |= (byte as u64) << s;
+                    let mut x = (ux >> 1) as i64;
                     if ux & 1 != 0 {
-                        x = !x
+                        x = -(x + 1)
                     }
                     return Ok(x);
                 }

diff --git a/decoder.test.ts b/decoder.test.ts
@@ -149,28 +149,34 @@ describe("Decoder", () => {
   });
 
   it("Can decode Int32", () => {
-    const expected = -2147483648;
+    const expected = 2147483647;
+    const expectedNegative = -2147483647;
 
-    const encoded = new Encoder().int32(expected).bytes;
+    const encoded = new Encoder().int32(expected).int32(expectedNegative).bytes;
     const decoder = new Decoder(encoded);
 
     const value = decoder.int32();
+    const valueNegative = decoder.int32();
 
     expect(value).toBe(expected);
+    expect(valueNegative).toBe(expectedNegative);
     expect(decoder.length).toBe(0);
 
     expect(() => decoder.int32()).toThrowError(InvalidInt32Error);
   });
 
   it("Can decode Int64", () => {
-    const expected = -9223372036854775808n;
+    const expected = 9223372036854775807n;
+    const expectedNegative = -9223372036854775807n;
 
-    const encoded = new Encoder().int64(expected).bytes;
+    const encoded = new Encoder().int64(expected).int64(expectedNegative).bytes;
     const decoder = new Decoder(encoded);
 
     const value = decoder.int64();
+    const valueNegative = decoder.int64();
 
     expect(value).toBe(expected);
+    expect(valueNegative).toBe(expectedNegative);
     expect(decoder.length).toBe(0);
 
     expect(() => decoder.int64()).toThrowError(InvalidInt64Error);