Fix infinite loop and resource exhaustion caused by crafted msg file

package.json

@@ -8,7 +8,7 @@
   "scripts": {
     "build": "tsc",
     "clean": "rimraf lib",
-    "test": "npm run mocha test/ test2/",
+    "test": "npm run mocha test/ test2/ test3/",
     "prepare": "npm run clean && npm run build && npm run test",
     "mocha": "set NODE_ENV=test && mocha",
     "dr": "npm publish --dry-run"
@@ -18,6 +18,7 @@
   "license": "Apache-2.0",
   "devDependencies": {
     "@types/node": "^26.3.0",
+    "chai": "^6.3.0",
     "commander": "^15.0.0",
     "mocha": "^12.0.2",
     "rimraf": "^6.1.3",

src/Burner.ts

@@ -101,7 +101,7 @@ function repeatValue(value: number, count: number): number[] {
 class LiteFat {
     sectors: number[];
 
-    constructor(source) {
+    constructor(source: number[]) {
         this.sectors = source;
     }
 
@@ -233,7 +233,7 @@ class LiteBurner {
             }
 
             while (true) {
-                const remain = (difat2.length & 127);
+                const remain: number = (difat2.length & 127);
                 if (remain === 0) {
                     break;
                 }
@@ -280,7 +280,7 @@ class LiteBurner {
             const pos = 512 * (1 + entriesFirstSector) + 128 * x;
 
             ds.seek(pos);
-            ds.writeUCS2String(liteEnt.entry.name, null, null);
+            ds.writeUCS2String(liteEnt.entry.name, undefined, undefined);
             const numBytesName = ds.position - pos;
 
             ds.seek(pos + 0x40);
@@ -314,7 +314,7 @@ class LiteBurner {
                 && it.isMini === false
             )
         ) {
-            const bytes = liteEnt.entry.binaryProvider();
+            const bytes = liteEnt.entry.binaryProvider && liteEnt.entry.binaryProvider() || [];
             ds.seek(512 * (1 + liteEnt.firstSector));
             ds.writeUint8Array(bytes);
         }
@@ -325,7 +325,7 @@ class LiteBurner {
                 && it.isMini === true
             )
         ) {
-            const bytes = liteEnt.entry.binaryProvider();
+            const bytes = liteEnt.entry.binaryProvider && liteEnt.entry.binaryProvider() || [];
             ds.seek(512 * (1 + firstMiniDataSector) + 64 * liteEnt.firstSector);
             ds.writeUint8Array(bytes);
         }
@@ -387,7 +387,7 @@ class LiteBurner {
         }
 
         // Array.sort is destructive, so copy it by concat() before changing
-        const children = liteEntry.entry.children.concat();
+        const children = (liteEntry.entry.children || []).concat();
         if (1 <= children.length) {
             children.sort(
                 (a, b) => {
@@ -401,7 +401,7 @@ class LiteBurner {
             // (     | 0   )
             // (   0 | 1   )
             // (   0 | 1 2 )
- 
+
             // (left, right), returns first right node
             const split2 = (start: number, end: number, isRed: boolean): number => {
                 if (start < end) {

src/MsgReader.ts

@@ -20,7 +20,7 @@ import CONST from './const'
 import DataStream from './DataStream'
 import { CFileSet, CFolder, Reader } from './Reader';
 import { burn, Entry } from './Burner';
-import { bin2HexUpper, emptyToNull, msftUuidStringify, toHex2, toHex4 } from './utils';
+import { bin2HexUpper, emptyToUndefined, msftUuidStringify, toHex2, toHex4 } from './utils';
 import { parse as entryStreamParser } from './EntryStreamParser';
 import { parse as parseVerbStream } from './VerbStreamParser';
 import { parse as parseTZDEFINITION, TzDefinition } from './TZDEFINITIONParser';
@@ -1926,7 +1926,7 @@ export default class MsgReader {
         {
           propertyObserver: (this.parserConfig?.propertyObserver) || (() => { }),
           includeRawProps: this.parserConfig?.includeRawProps ? true : false,
-          ansiEncoding: emptyToNull(this.parserConfig?.ansiEncoding),
+          ansiEncoding: emptyToUndefined(this.parserConfig?.ansiEncoding),
         }
       );
     }

src/Reader.ts

@@ -1,5 +1,5 @@
 import DataStream from "./DataStream";
-import { arraysEqual } from "./utils";
+import { arraysEqual, createVisitTester } from "./utils";
 import CONST from './const'
 
 /**
@@ -129,13 +129,27 @@ export class Reader {
   private xbatStart: number;
   private xbatCount: number;
 
-  private batData?: number[];
-  private sbatData?: number[];
-  private propertyData?: Property[];
+  private batData: number[];
+  private sbatData: number[];
+  private propertyData: Property[];
   private bigBlockTable: number[];
 
   constructor(arrayBuffer: ArrayBuffer | DataView) {
     this.ds = new DataStream(arrayBuffer, 0, DataStream.LITTLE_ENDIAN);
+
+    this.bigBlockSize = 0;
+    this.bigBlockLength = 0;
+    this.xBlockLength = 0;
+    this.batCount = 0;
+    this.propertyStart = 0;
+    this.sbatStart = 0;
+    this.sbatCount = 0;
+    this.xbatStart = 0;
+    this.xbatCount = 0;
+    this.batData = [];
+    this.sbatData = [];
+    this.propertyData = [];
+    this.bigBlockTable = [];
   }
 
   isMSGFile(): boolean {
@@ -222,9 +236,10 @@ export class Reader {
     }
     nodeProperty.children = [];
 
-    const children = [{ currentMode: "walk", currentIndex: nodeProperty.childProperty }];
+    const visit = createVisitTester(new Error("Cyclic property hierarchy detected"));
+    const children: { currentMode: string, currentIndex: number }[] = [{ currentMode: "walk", currentIndex: nodeProperty.childProperty }];
     while (children.length != 0) {
-      const { currentMode, currentIndex } = children.pop();
+      const { currentMode, currentIndex } = children.pop() || (() => { throw Error(); })();
       const current = props[currentIndex];
 
       if (currentMode === "push") {
@@ -239,6 +254,7 @@ export class Reader {
           children.push({ currentMode: "walk", currentIndex: current.nextProperty, });
         }
 
+        visit(currentIndex);
         children.push({ currentMode: "push", currentIndex: currentIndex, });
 
         if (current.previousProperty != CONST.MSG.PROP.NO_INDEX) {
@@ -252,9 +268,12 @@ export class Reader {
   private propertyDataReader(propertyStart: number): Property[] {
     const props: Property[] = [];
 
+    const visit = createVisitTester(new Error("Cyclic property chain detected"));
+
     let currentOffset = propertyStart;
 
     while (currentOffset != CONST.MSG.END_OF_CHAIN) {
+      visit(currentOffset);
       this.convertBlockToProperties(currentOffset, props);
       currentOffset = this.getNextBlock(currentOffset);
     }
@@ -325,8 +344,10 @@ export class Reader {
   private sbatDataReader(): number[] {
     const result = [];
     let startIndex = this.sbatStart;
+    const visit = createVisitTester(new Error("Cyclic small block chain detected"));
 
     for (let i = 0; i < this.sbatCount && startIndex && startIndex != CONST.MSG.END_OF_CHAIN; i++) {
+      visit(startIndex);
       result.push(startIndex);
       startIndex = this.getNextBlock(startIndex);
     }
@@ -365,8 +386,10 @@ export class Reader {
 
   private getChainByBlockSmall(fieldProperty: Property): number[] {
     const blockChain = [];
+    const visit = createVisitTester(new Error("Cyclic small block chain detected"));
     let nextBlockSmall = fieldProperty.startBlock;
     while (nextBlockSmall != CONST.MSG.END_OF_CHAIN) {
+      visit(nextBlockSmall);
       blockChain.push(nextBlockSmall);
       nextBlockSmall = this.getNextBlockSmall(nextBlockSmall);
     }
@@ -376,8 +399,10 @@ export class Reader {
   private readBigBlockTable(): number[] {
     const rootProp = this.propertyData[0];
     const table: number[] = [];
+    const visit = createVisitTester(new Error("Cyclic big block chain detected"));
     let nextBlock = rootProp.startBlock;
-    for (let i = 0; nextBlock != CONST.MSG.END_OF_CHAIN; i++) {
+    for (; nextBlock != CONST.MSG.END_OF_CHAIN;) {
+      visit(nextBlock);
       table.push(nextBlock);
       nextBlock = this.getNextBlock(nextBlock);
     }
@@ -431,6 +456,7 @@ export class Reader {
       let nextBlock = fieldProperty.startBlock;
       let remaining = fieldProperty.sizeBlock;
       let position = 0;
+      const visit = createVisitTester(new Error("Cyclic big block chain detected"));
       const resultData = new Uint8Array(fieldProperty.sizeBlock);
       while (1 <= remaining) {
         const blockStartOffset = this.getBlockOffsetAt(nextBlock);
@@ -440,6 +466,7 @@ export class Reader {
         resultData.set(part, position);
         position += partSize;
         remaining -= partSize;
+        visit(nextBlock);
         nextBlock = this.getNextBlock(nextBlock);
       }
       return resultData;
@@ -503,7 +530,8 @@ export class Reader {
         if (children) {
           return children
             .filter(subIndex => propertyData[subIndex].type == TypeEnum.DIRECTORY)
-            .map(subIndex => this.folderOf(subIndex));
+            .map(subIndex => this.folderOf(subIndex))
+            .filter(it => it !== null);
         }
         return [];
       },
@@ -528,6 +556,6 @@ export class Reader {
    * @returns root folder
    */
   rootFolder(): CFolder {
-    return this.folderOf(0);
+    return this.folderOf(0)!;
   }
 }

src/utils.ts

@@ -1,4 +1,3 @@
-import { stringify } from "querystring";
 import DataStream from "./DataStream";
 import { TransitionSystemTime } from "./TZDEFINITIONParser";
 
@@ -122,16 +121,20 @@ export function msftUuidStringify(array: ArrayLike<number>, offset: number): str
 /**
  * @internal
  */
-export function emptyToNull(text: string): string {
-  return (text === "") ? null : text;
+export function emptyToUndefined(text: string): string | undefined {
+  return (text === "") ? undefined : text;
 }
 
 
 /**
  * @internal
  */
 function padNumber(value: number, maxLen: number): string {
-  return ("" + value).padStart(maxLen, '0');
+  let text = "" + value;
+  while (text.length < maxLen) {
+    text = '0' + text;
+  }
+  return text;
 }
 
 /**
@@ -195,3 +198,18 @@ export function bin2HexUpper(ds: DataStream): string {
   }
   return text.toUpperCase();
 }
+
+/**
+ * @internal
+ */
+export function createVisitTester(throwErrorOnDuplicate: Error): (value: number) => void {
+  const set = new Set<number>();
+  return (value: number) => {
+    if (set.has(value)) {
+      throw throwErrorOnDuplicate;
+    }
+    else {
+      set.add(value);
+    }
+  };
+}

test3/test.js

@@ -0,0 +1,20 @@
+const fs = require('fs');
+const chai = require('chai');
+const expect = chai.expect;
+
+describe('ReaderAntiCyclic', function () {
+  const Reader = require('../lib/Reader').Reader;
+
+  function testIt(file) {
+    const array = fs.readFileSync(`test3/${file}`);
+    const reader = new Reader(array);
+    reader.parse();
+  };
+
+  it('cyclicDirTree.msg', function () {
+    expect(testIt.bind(null, 'cyclicDirTree.msg')).to.throw("Cyclic property hierarchy detected");
+  });
+  it('cyclicFatRing.msg', function () {
+    expect(testIt.bind(null, 'cyclicFatRing.msg')).to.throw("Cyclic property chain detected");
+  });
+});

yarn.lock

@@ -112,6 +112,11 @@ camelcase@^8.0.0:
   resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-8.0.0.tgz#c0d36d418753fb6ad9c5e0437579745c1c14a534"
   integrity sha512-8WB3Jcas3swSvjIeA2yvCJ+Miyz5l1ZmB6HFb9R1317dt9LCQoswg/BGrmAmkWVEszSrrg4RwmO46qIm2OEnSA==
 
+chai@^6.3.0:
+  version "6.3.0"
+  resolved "https://registry.yarnpkg.com/chai/-/chai-6.3.0.tgz#91f89eb789c2bb1ae1787d78c8518e1d3cebfc07"
+  integrity sha512-XWAtwJ6OHO+tj0EKCs0Y2UamnyOxseZWltU4x2U2wh8g4AigdjwvtUjvLP2tqkA/avxHEtzxNaqGq/YGNwckKg==
+
 chokidar@^5.0.0:
   version "5.0.0"
   resolved "https://registry.yarnpkg.com/chokidar/-/chokidar-5.0.0.tgz#949c126a9238a80792be9a0265934f098af369a5"