From 0eb493ce4466f8e8f60f13a7076ab43ada71e2db Mon Sep 17 00:00:00 2001 From: Martin Haug Date: Thu, 8 May 2025 22:24:02 +0200 Subject: [PATCH] [decoding] add limits to variable-length decoders This commit allows decoder users to limit how many characters or array elements they want to decode. Such limits are useful in a server environment because `lib0` synchronously decodes messages. An attacker just send a message with a very long string or array and stall the main thread of the server so that no other requests can be served. The length limits have been added to these public functions: - `readVarUint8Array` - `readTerminatedUint8Array` - `readVarString` - `peekVarString` - `readTerminatedString` - The constructor of `StringDecoder` Each of these functions accepts a new trailing optional argument `maxLen`. The change should therefore not be breaking. I have added some tests, the code is fully covered. --- decoding.js | 41 +++++++++++++++++++++++++++--------- encoding.test.js | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/decoding.js b/decoding.js index c88f9f1..db224cb 100644 --- a/decoding.js +++ b/decoding.js @@ -35,6 +35,7 @@ import * as encoding from './encoding.js' const errorUnexpectedEndOfArray = error.create('Unexpected end of array') const errorIntegerOutOfRange = error.create('Integer out of Range') +const errorValueTooLong = error.create('Value too long') /** * A Decoder handles the decoding of an Uint8Array. @@ -113,9 +114,17 @@ export const readUint8Array = (decoder, len) => { * * @function * @param {Decoder} decoder + * @param {number} [maxLen] Maximum length of the array * @return {Uint8Array} */ -export const readVarUint8Array = decoder => readUint8Array(decoder, readVarUint(decoder)) +export const readVarUint8Array = (decoder, maxLen) => { + const len = readVarUint(decoder) + if (maxLen !== undefined && len > maxLen) { + throw errorValueTooLong + } + + return readUint8Array(decoder, len) +} /** * Read the rest of the content as an ArrayBuffer @@ -336,13 +345,16 @@ export const peekVarInt = decoder => { * * @function * @param {Decoder} decoder + * @param {number} [maxLen] Maximum length of the string in bytes * @return {String} The read String. */ /* c8 ignore start */ -export const _readVarStringPolyfill = decoder => { +export const _readVarStringPolyfill = (decoder, maxLen) => { let remainingLen = readVarUint(decoder) if (remainingLen === 0) { return '' + } else if (maxLen !== undefined && remainingLen > maxLen) { + throw errorValueTooLong } else { let encodedString = String.fromCodePoint(readUint8(decoder)) // remember to decrease remainingLen if (--remainingLen < 100) { // do not create a Uint8Array for small strings @@ -368,10 +380,11 @@ export const _readVarStringPolyfill = decoder => { /** * @function * @param {Decoder} decoder + * @param {number} [maxLen] Maximum length of the string in bytes * @return {String} The read String */ -export const _readVarStringNative = decoder => - /** @type any */ (string.utf8TextDecoder).decode(readVarUint8Array(decoder)) +export const _readVarStringNative = (decoder, maxLen) => + /** @type any */ (string.utf8TextDecoder).decode(readVarUint8Array(decoder, maxLen)) /** * Read string of variable length @@ -379,6 +392,7 @@ export const _readVarStringNative = decoder => * * @function * @param {Decoder} decoder + * @param {number} [maxLen] Maximum length of the string in bytes * @return {String} The read String * */ @@ -387,12 +401,16 @@ export const readVarString = string.utf8TextDecoder ? _readVarStringNative : _re /** * @param {Decoder} decoder + * @param {number} [maxLen] Maximum length of the array * @return {Uint8Array} */ -export const readTerminatedUint8Array = decoder => { +export const readTerminatedUint8Array = (decoder, maxLen) => { const encoder = encoding.createEncoder() let b while (true) { + if (maxLen !== undefined && encoding.length(encoder) > maxLen) { + throw errorValueTooLong + } b = readUint8(decoder) if (b === 0) { return encoding.toUint8Array(encoder) @@ -406,20 +424,22 @@ export const readTerminatedUint8Array = decoder => { /** * @param {Decoder} decoder + * @param {number} [maxLen] Maximum length of the array string in bytes * @return {string} */ -export const readTerminatedString = decoder => string.decodeUtf8(readTerminatedUint8Array(decoder)) +export const readTerminatedString = (decoder, maxLen) => string.decodeUtf8(readTerminatedUint8Array(decoder, maxLen)) /** * Look ahead and read varString without incrementing position * * @function * @param {Decoder} decoder + * @param {number} [maxLen] Maximum length of the array string in bytes * @return {string} */ -export const peekVarString = decoder => { +export const peekVarString = (decoder, maxLen) => { const pos = decoder.pos - const s = readVarString(decoder) + const s = readVarString(decoder, maxLen) decoder.pos = pos return s } @@ -684,10 +704,11 @@ export class IntDiffOptRleDecoder extends Decoder { export class StringDecoder { /** * @param {Uint8Array} uint8Array + * @param {number} [maxLen] Maximum length of the string in bytes */ - constructor (uint8Array) { + constructor (uint8Array, maxLen) { this.decoder = new UintOptRleDecoder(uint8Array) - this.str = readVarString(this.decoder) + this.str = readVarString(this.decoder, maxLen) /** * @type {number} */ diff --git a/encoding.test.js b/encoding.test.js index 179bb02..264b531 100644 --- a/encoding.test.js +++ b/encoding.test.js @@ -802,6 +802,22 @@ export const testStringDecoder = tc => { } } +/** + * @param {t.TestCase} _tc + */ +export const testStringDecoderLengthLimit = _tc => { + const tooLongText = 'a'.repeat(100) + const encoderA = new encoding.StringEncoder() + encoderA.write(tooLongText) + t.fails(() => new decoding.StringDecoder(encoderA.toUint8Array(), 10)) + + const okayText = 'a'.repeat(9) + const encoderB = new encoding.StringEncoder() + encoderB.write(okayText) + const decoder = new decoding.StringDecoder(encoderB.toUint8Array(), 10) + t.assert(decoder.read() === okayText) +} + /** * @param {t.TestCase} tc */ @@ -869,3 +885,41 @@ export const testTerminatedEncodering = _tc => { t.compare(readBuf1, buf1) t.compare(readBuf2, buf2) } + +/** + * @param {t.TestCase} _tc + */ +export const testVarUint8ArrayLengthLimitEncoding = _tc => { + const buf1 = new Uint8Array([0, 1, 2, 255, 4, 5]) + const buf2 = new Uint8Array([255, 255, 0, 0, 0, 1, 0, 0]) + + const encoder = encoding.createEncoder() + encoding.writeVarUint8Array(encoder, buf1) + encoding.writeVarUint8Array(encoder, buf2) + + const decoder = decoding.createDecoder(encoding.toUint8Array(encoder)) + const readBuf1 = decoding.readVarUint8Array(decoder, 6) + t.compare(readBuf1, buf1) + t.fails(() => { + decoding.readVarUint8Array(decoder, 6) + }) +} + +/** + * @param {t.TestCase} _tc + */ +export const testTerminatedUint8ArrayLengthLimitEncoding = _tc => { + const buf1 = new Uint8Array([0, 1, 2, 255, 4, 5]) + const buf2 = new Uint8Array([255, 255, 0, 0, 0, 1, 0, 0]) + + const encoder = encoding.createEncoder() + encoding.writeTerminatedUint8Array(encoder, buf1) + encoding.writeTerminatedUint8Array(encoder, buf2) + + const decoder = decoding.createDecoder(encoding.toUint8Array(encoder)) + const readBuf1 = decoding.readTerminatedUint8Array(decoder, 6) + t.compare(readBuf1, buf1) + t.fails(() => { + decoding.readTerminatedUint8Array(decoder, 6) + }) +}