Merge pull request #2143 from microsoft/connor4312/vs-2266626

fix: content verification of files in Node.js failing with UTF-8 BOM

Author: connor4312

package-lock.json

@@ -50,7 +50,7 @@
     "test:lint": "gulp lint"
   },
   "dependencies": {
-    "@c4312/chromehash": "^0.3.0",
+    "@c4312/chromehash": "^0.3.1",
     "@jridgewell/gen-mapping": "^0.3.3",
     "@jridgewell/trace-mapping": "^0.3.22",
     "@vscode/js-debug-browsers": "^1.1.2",

package.json

@@ -1,7 +1,12 @@
 /*---------------------------------------------------------
  * Copyright (C) Microsoft Corporation. All rights reserved.
  *--------------------------------------------------------*/
-import { hash, shaHash } from '@c4312/chromehash';
+import {
+  hash as chromeHash,
+  normalizeShaBuffer,
+  shaHash as chromeShaHash,
+} from '@c4312/chromehash';
+import { createHash } from 'crypto';
 import { promises as fs } from 'fs';
 import { MessagePort, parentPort } from 'worker_threads';
 
@@ -13,17 +18,29 @@ export const enum MessageType {
 }
 
 export const enum HashMode {
+  // Legacy hash mode, pre-https://chromium-review.googlesource.com/c/v8/v8/+/3229957
   Chromehash,
+  // BOM-aware hash mode, used by Chrome/Browsers. (Hashes the contents normalized to UTF-8)
   SHA256,
+  // Naive hash mode, used by Node.js. (Hashes the raw file bytes)
+  SHA256Naive,
 }
 
+export const shaHashNaive = (input: Buffer) => createHash('sha256').update(input).digest('hex');
+
 /**
  * Message sent to the hash worker.
  */
 export type HashRequest =
   | { type: MessageType.HashFile; id: number; file: string; mode: HashMode }
   | { type: MessageType.HashBytes; id: number; data: string | Buffer; mode: HashMode }
-  | { type: MessageType.VerifyFile; id: number; file: string; expected: string; checkNode: boolean }
+  | {
+    type: MessageType.VerifyFile;
+    id: number;
+    file: string;
+    expected: string;
+    checkNode: boolean;
+  }
   | {
     type: MessageType.VerifyBytes;
     id: number;
@@ -74,9 +91,33 @@ const LF = Buffer.from('\n')[0];
 
 const hasPrefix = (buf: Buffer, prefix: Buffer) => buf.slice(0, prefix.length).equals(prefix);
 
+const verifyHash = (expected: string, ...data: Buffer[]) => {
+  if (expected.length !== 64) {
+    return chromeHash(data.length === 1 ? data[0] : Buffer.concat(data)) === expected;
+  }
+
+  // Check if the non-normalized hash matches first. We check both because we
+  // want to check with both BOM-normalization (used in Chrome) and
+  // without it (used in Node.js).
+  const nonNormalizedHash = createHash('sha256');
+  for (const d of data) {
+    nonNormalizedHash.update(d);
+  }
+  if (nonNormalizedHash.digest('hex') === expected) {
+    return true;
+  }
+
+  const normalizedInput = data.length === 1 ? data[0] : Buffer.concat(data);
+  const normalizedOutput = normalizeShaBuffer(normalizedInput);
+  if (normalizedInput === normalizedOutput) {
+    return false;
+  }
+
+  return createHash('sha256').update(normalizedOutput).digest('hex') === expected;
+};
+
 const verifyBytes = (bytes: Buffer, expected: string, checkNode: boolean) => {
-  const hashFn = expected.length === 64 ? shaHash : hash;
-  if (hashFn(bytes) === expected) {
+  if (verifyHash(expected, bytes)) {
     return true;
   }
 
@@ -88,16 +129,16 @@ const verifyBytes = (bytes: Buffer, expected: string, checkNode: boolean) => {
         end--;
       }
 
-      return hashFn(bytes.slice(end)) === expected;
+      return verifyHash(expected, bytes.subarray(end));
     }
 
-    if (hashFn(Buffer.concat([nodePrefix, bytes, nodeSuffix])) === expected) {
+    if (verifyHash(expected, nodePrefix, bytes, nodeSuffix)) {
       return true;
     }
   }
 
   // todo -- doing a lot of concats, make chromehash able to hash an iterable of buffers?
-  if (hashFn(Buffer.concat([electronPrefix, bytes, electronSuffix])) === expected) {
+  if (verifyHash(expected, electronPrefix, bytes, electronSuffix)) {
     return true;
   }
 
@@ -114,14 +155,18 @@ async function handle(message: HashRequest): Promise<HashResponse<HashRequest>>
         const data = await fs.readFile(message.file);
         return {
           id: message.id,
-          hash: message.mode === HashMode.Chromehash ? hash(data) : shaHash(data),
+          hash: message.mode === HashMode.Chromehash
+            ? chromeHash(data)
+            : message.mode === HashMode.SHA256Naive
+            ? shaHashNaive(data)
+            : chromeShaHash(data),
         };
       } catch (e) {
         return { id: message.id };
       }
     case MessageType.HashBytes:
       try {
-        return { id: message.id, hash: hash(toBuffer(message.data)) };
+        return { id: message.id, hash: chromeHash(toBuffer(message.data)) };
       } catch (e) {
         return { id: message.id };
       }

src/common/hash/hash.ts

@@ -163,6 +163,15 @@ describe('hash process', function() {
       );
       expect(b).to.be.false;
     });
+
+    it('verifies raw content for BOM-less Node', async () => {
+      const a = await hasher.verifyFile(
+        join(hashTestCaseDir, 'utf8-bom.js'),
+        '027cd940c96749db7c716eabe8e09ad578a3c0b5dfc821dc2bb67f627be70ff1',
+        false,
+      );
+      expect(a).to.be.true;
+    });
   });
 
   it('verifies if wrapped in node module', async () => {