From d88422fc54ce53d4209b6e9a9b5dc3e9cf84bf24 Mon Sep 17 00:00:00 2001
From: gvergnaud <gabriel.vergnaud@gmail.com>
Date: Thu, 9 Jan 2025 15:40:05 -0500
Subject: [PATCH] security: add update size limits to prevent DOS attacks

---
 src/utils/encoding.js |  5 +++
 src/utils/limits.js   | 48 ++++++++++++++++++++++
 src/utils/updates.js  |  5 +++
 tests/index.js        |  3 +-
 tests/limits.tests.js | 93 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 153 insertions(+), 1 deletion(-)
 create mode 100644 src/utils/limits.js
 create mode 100644 tests/limits.tests.js

diff --git a/src/utils/encoding.js b/src/utils/encoding.js
index b195ccc3..4e59002e 100644
--- a/src/utils/encoding.js
+++ b/src/utils/encoding.js
@@ -45,6 +45,7 @@ import * as binary from 'lib0/binary'
 import * as map from 'lib0/map'
 import * as math from 'lib0/math'
 import * as array from 'lib0/array'
+import { assertMaxGCLength, assertMaxSkipLength, assertMaxStructs, assertMaxUpdates } from './limits.js'
 
 /**
  * @param {UpdateEncoderV1 | UpdateEncoderV2} encoder
@@ -115,8 +116,10 @@ export const readClientsStructRefs = (decoder, doc) => {
    */
   const clientRefs = map.create()
   const numOfStateUpdates = decoding.readVarUint(decoder.restDecoder)
+  assertMaxUpdates(numOfStateUpdates)
   for (let i = 0; i < numOfStateUpdates; i++) {
     const numberOfStructs = decoding.readVarUint(decoder.restDecoder)
+    assertMaxStructs(numberOfStructs)
     /**
      * @type {Array<GC|Item>}
      */
@@ -130,6 +133,7 @@ export const readClientsStructRefs = (decoder, doc) => {
       switch (binary.BITS5 & info) {
         case 0: { // GC
           const len = decoder.readLen()
+          assertMaxGCLength(len)
           refs[i] = new GC(createID(client, clock), len)
           clock += len
           break
@@ -137,6 +141,7 @@ export const readClientsStructRefs = (decoder, doc) => {
         case 10: { // Skip Struct (nothing to apply)
           // @todo we could reduce the amount of checks by adding Skip struct to clientRefs so we know that something is missing.
           const len = decoding.readVarUint(decoder.restDecoder)
+          assertMaxSkipLength(len)
           refs[i] = new Skip(createID(client, clock), len)
           clock += len
           break
diff --git a/src/utils/limits.js b/src/utils/limits.js
new file mode 100644
index 00000000..877d873d
--- /dev/null
+++ b/src/utils/limits.js
@@ -0,0 +1,48 @@
+export const MAX_STRUCTS = 100_000
+export const MAX_UPDATES = 100_000
+export const MAX_GC_LENGTH = 100_000
+export const MAX_SKIP_LENGTH = 100_000
+
+/**
+ * @param {number} numOfStateUpdates
+ */
+export function assertMaxUpdates (numOfStateUpdates) {
+  if (numOfStateUpdates > MAX_UPDATES) {
+    throw new Error(
+      `This update exceeds the maximum number of updates. ${numOfStateUpdates} > ${MAX_UPDATES}`
+    )
+  }
+}
+
+/**
+ * @param {number} numberOfStructs
+ */
+export function assertMaxStructs (numberOfStructs) {
+  if (numberOfStructs > MAX_STRUCTS) {
+    throw new Error(
+      `This update exceeds the maximum number of structs. ${numberOfStructs} > ${MAX_STRUCTS}`
+    )
+  }
+}
+
+/**
+ * @param {number} len
+ */
+export function assertMaxSkipLength (len) {
+  if (len > MAX_SKIP_LENGTH) {
+    throw new Error(
+      `This skip length exceeds the limit. ${len} > ${MAX_SKIP_LENGTH}`
+    )
+  }
+}
+
+/**
+ * @param {number} len
+ */
+export function assertMaxGCLength (len) {
+  if (len > MAX_GC_LENGTH) {
+    throw new Error(
+      `This garbage collection update's length exceeds the limit. ${len} > ${MAX_GC_LENGTH}`
+    )
+  }
+}
diff --git a/src/utils/updates.js b/src/utils/updates.js
index fc40cd57..f93752b2 100644
--- a/src/utils/updates.js
+++ b/src/utils/updates.js
@@ -36,21 +36,25 @@ import {
   YXmlElement,
   YXmlHook
 } from '../internals.js'
+import { assertMaxGCLength, assertMaxSkipLength, assertMaxStructs, assertMaxUpdates } from './limits.js'
 
 /**
  * @param {UpdateDecoderV1 | UpdateDecoderV2} decoder
  */
 function * lazyStructReaderGenerator (decoder) {
   const numOfStateUpdates = decoding.readVarUint(decoder.restDecoder)
+  assertMaxUpdates(numOfStateUpdates)
   for (let i = 0; i < numOfStateUpdates; i++) {
     const numberOfStructs = decoding.readVarUint(decoder.restDecoder)
     const client = decoder.readClient()
     let clock = decoding.readVarUint(decoder.restDecoder)
+    assertMaxStructs(numberOfStructs)
     for (let i = 0; i < numberOfStructs; i++) {
       const info = decoder.readInfo()
       // @todo use switch instead of ifs
       if (info === 10) {
         const len = decoding.readVarUint(decoder.restDecoder)
+        assertMaxSkipLength(len)
         yield new Skip(createID(client, clock), len)
         clock += len
       } else if ((binary.BITS5 & info) !== 0) {
@@ -74,6 +78,7 @@ function * lazyStructReaderGenerator (decoder) {
         clock += struct.length
       } else {
         const len = decoder.readLen()
+        assertMaxGCLength(len)
         yield new GC(createID(client, clock), len)
         clock += len
       }
diff --git a/tests/index.js b/tests/index.js
index fd9c08e7..8c6d820c 100644
--- a/tests/index.js
+++ b/tests/index.js
@@ -11,6 +11,7 @@ import * as doc from './doc.tests.js'
 import * as snapshot from './snapshot.tests.js'
 import * as updates from './updates.tests.js'
 import * as relativePositions from './relativePositions.tests.js'
+import * as limits from './limits.tests.js'
 
 import { runTests } from 'lib0/testing'
 import { isBrowser, isNode } from 'lib0/environment'
@@ -25,7 +26,7 @@ if (isBrowser) {
  * @type {any}
  */
 const tests = {
-  doc, map, array, text, xml, encoding, undoredo, compatibility, snapshot, updates, relativePositions
+  doc, map, array, text, xml, encoding, undoredo, compatibility, snapshot, updates, relativePositions, limits
 }
 
 const run = async () => {
diff --git a/tests/limits.tests.js b/tests/limits.tests.js
new file mode 100644
index 00000000..5f1d60aa
--- /dev/null
+++ b/tests/limits.tests.js
@@ -0,0 +1,93 @@
+import * as t from 'lib0/testing'
+import * as Y from '../src/index.js'
+
+/**
+ * @param {() => void} f function that should throw
+ * @returns {boolean}
+ */
+const shouldThrow = (f) => {
+  try {
+    f()
+    return false
+  } catch (/** @type {any} */ e) {
+    console.log('Error thrown:', e?.message)
+    return true
+  }
+}
+
+/**
+ * @param {t.TestCase} _tc
+ */
+export const testShouldntMergeUpdatesWithTooManyStructs = (_tc) => {
+  // Binary with 4398046511101 structs.
+  const buf = new Uint8Array([
+    0, 1, 0, 0, 0, 0, 1, 0, 1, 0, 1, 2, 0, 0, 22, 2, 0, 0, 1, 253, 255, 255,
+    255, 255, 127, 0, 0
+  ])
+
+  t.assert(shouldThrow(() => {
+    const update = Y.encodeStateAsUpdateV2(new Y.Doc())
+    Y.mergeUpdatesV2([update, buf])
+  }))
+}
+
+/**
+ * @param {t.TestCase} _tc
+ */
+export const testShouldntMergeUpdatesWithTooManyUpdatesV1 = (_tc) => {
+  // Binary with 265828 updates.
+  const buf = new Uint8Array([
+    228, 156, 16, 0, 5, 255, 255, 5, 0, 1, 0, 0, 0, 1, 1, 0, 0, 0, 237,
+    0, 0, 0, 1, 1, 0, 254, 184, 194, 233, 173, 135, 217, 18, 0, 0, 1, 1,
+    255, 237, 246
+  ])
+  const update = Y.encodeStateAsUpdate(new Y.Doc())
+  t.assert(shouldThrow(() => {
+    Y.mergeUpdates([update, buf])
+  }))
+}
+
+/**
+ * @param {t.TestCase} _tc
+ */
+export const testShouldntMergeUpdatesWithTooManyUpdatesV2 = (_tc) => {
+  // Binary with 7658324286 updates.
+  const buf = new Uint8Array([
+    228, 149, 0, 0, 1, 24, 0, 0, 1, 0, 1, 0, 0, 1, 0, 0, 237, 1, 190,
+    130, 227, 195, 28, 1, 2, 228, 149, 0, 0, 1, 24, 0, 0, 1, 24, 0, 0,
+    1, 0, 1, 237, 0
+  ])
+  const update = Y.encodeStateAsUpdateV2(new Y.Doc())
+
+  t.assert(shouldThrow(() => {
+    Y.mergeUpdatesV2([update, buf])
+  }))
+}
+
+/**
+ * @param {t.TestCase} _tc
+ */
+export const testShouldntAcceptTooLargeGCItems = (_tc) => {
+  // Update with a GC item of length 553253.
+  const buf = new Uint8Array([
+    143, 1, 128, 0, 0, 0, 1, 170, 1, 0, 1, 2, 0, 0, 22, 2, 0, 229, 196,
+    67, 20, 231, 166, 139, 147, 174, 181, 253, 93, 232, 38, 154, 138,
+    89, 0, 49, 213, 15, 18, 1, 48, 0, 0, 0
+  ])
+  const update = Y.encodeStateAsUpdateV2(new Y.Doc())
+  t.assert(shouldThrow(() => {
+    Y.mergeUpdatesV2([update, buf])
+  }))
+}
+
+export const testShouldntApplyUpdatesOverLimit = () => {
+  // Binary with 267854847 structs in one update.
+  const buf = new Uint8Array([
+    0, 1, 35, 0, 0, 0, 1, 2, 129, 0, 0, 16, 0, 199, 220, 0, 196, 122, 128,
+    0, 65, 171, 234, 214, 0, 1, 0, 0, 1, 0, 0, 132, 0, 0, 16, 255, 199, 220,
+    255, 0, 0, 0
+  ])
+  t.assert(shouldThrow(() => {
+    Y.applyUpdateV2(new Y.Doc(), buf)
+  }))
+}