fix: reject non-minimally encoded varints (#343)

Per the unsigned-varint spec, varints MUST be minimally encoded and
MUST NOT exceed 9 bytes. `varint.decode` accepted non-minimal
encodings such as `[0x81, 0x00]` for `1`, making CID and multihash
prefixes malleable. Reject redundant final zero payload groups and
varints longer than 9 bytes.

Author: spokodev

src/varint.ts

@@ -2,7 +2,14 @@ import varint from './vendor/varint.js'
 
 export function decode (data: Uint8Array, offset = 0): [number, number] {
   const code = varint.decode(data, offset)
-  return [code, varint.decode.bytes]
+  const length = varint.decode.bytes
+  if (length > 9) {
+    throw new RangeError('Invalid varint: too long')
+  }
+  if (length > 1 && data[offset + length - 1] === 0) {
+    throw new RangeError('Invalid varint: not minimally encoded')
+  }
+  return [code, length]
 }
 
 export function encodeTo <T extends ArrayBufferLike> (int: number, target: Uint8Array<T>, offset = 0): Uint8Array<T> {

test/test-cid.spec.ts

@@ -177,6 +177,14 @@ describe('CID', () => {
       assert.deepStrictEqual(cid.toString(), cidStr)
     })
 
+    it('rejects a non-minimally encoded varint prefix', () => {
+      // the version varint 0x01 replaced by its non-minimal encoding 0x81 0x00
+      const cidBuf = fromHex(
+        '81007012207252523e6591fb8fe553d67ff55a86f84044b46a3e4176e10c58fa529a4aabd5'
+      )
+      assert.throws(() => CID.decode(cidBuf), /minimal/)
+    })
+
     it('create by parts', async () => {
       const hash = await sha256.digest(textEncoder.encode('abc'))
       const cid = CID.create(1, 0x71, hash)

test/test-varint.spec.ts

@@ -21,4 +21,28 @@ describe('varint', () => {
     assert.deepStrictEqual(varint.decode(bytes), [outerTag, outerTagSize])
     assert.deepStrictEqual(varint.decode(bytes, outerTagSize), [innerTag, innerTagSize])
   })
+
+  it('rejects non-minimally encoded varints', () => {
+    // 1 encodes minimally as [0x01]; [0x81, 0x00] is a non-minimal encoding of 1
+    assert.throws(() => varint.decode(Uint8Array.from([0x81, 0x00])), /minimal/)
+    // 0 encodes minimally as [0x00]; [0x80, 0x00] is a non-minimal encoding of 0
+    assert.throws(() => varint.decode(Uint8Array.from([0x80, 0x00])), /minimal/)
+    // non-minimal encoding at a non-zero offset
+    assert.throws(() => varint.decode(Uint8Array.from([0xff, 0x81, 0x00]), 1), /minimal/)
+    // 127 encodes minimally as [0x7f]; [0xff, 0x00] is a non-minimal encoding of 127
+    assert.throws(() => varint.decode(Uint8Array.from([0xff, 0x00])), /minimal/)
+  })
+
+  it('accepts interior zero groups', () => {
+    // 16384 = 1 << 14; the two leading groups are zero but the encoding is minimal
+    assert.deepStrictEqual(varint.decode(Uint8Array.from([0x80, 0x80, 0x01])), [16384, 3])
+  })
+
+  it('rejects varints longer than 9 bytes', () => {
+    // the unsigned-varint spec caps varints at 9 bytes (63 bits)
+    assert.throws(
+      () => varint.decode(Uint8Array.from([0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01])),
+      /too long/
+    )
+  })
 })