fix: Integrity#match prioritizes overlapping hashes

wraithgar · wraithgar · commit 4e94d15dd9ec · 2023-04-11T11:33:38.000-07:00
diff --git a/lib/index.js b/lib/index.js
@@ -51,15 +51,12 @@ class IntegrityStream extends MiniPass {
 
     if (!this.sri) {
       this.algorithm = null
-    } else if (this.sri.isIntegrity) {
-      this.goodSri = !this.sri.isEmpty()
-
-      if (this.goodSri) {
-        this.algorithm = this.sri.pickAlgorithm(this.opts)
-      }
     } else if (this.sri.isHash) {
       this.goodSri = true
       this.algorithm = this.sri.algorithm
+    } else {
+      this.goodSri = !this.sri.isEmpty()
+      this.algorithm = this.sri.pickAlgorithm(this.opts)
     }
 
     this.digests = this.goodSri ? this.sri[this.algorithm] : null
@@ -184,8 +181,13 @@ class Hash {
     if (!other) {
       return false
     }
-    if (other instanceof Integrity) {
-      const algo = other.pickAlgorithm(opts)
+    if (other.isIntegrity) {
+      const algo = other.pickAlgorithm(opts, [this.algorithm])
+
+      if (!algo) {
+        return false
+      }
+
       const foundHash = other[algo].find(hash => hash.digest === this.digest)
 
       if (foundHash) {
@@ -323,8 +325,9 @@ class Integrity {
     if (!other) {
       return false
     }
-    const algo = other.pickAlgorithm(opts)
+    const algo = other.pickAlgorithm(opts, Object.keys(this))
     return (
+      !!algo &&
       this[algo] &&
       other[algo] &&
       this[algo].find(hash =>
@@ -335,12 +338,22 @@ class Integrity {
     ) || false
   }
 
-  pickAlgorithm (opts) {
+  // Pick the highest priority algorithm present, optionally also limited to a
+  // set of hashes found in another integrity.  When limiting it may return
+  // nothing.
+  pickAlgorithm (opts, hashes) {
     const pickAlgorithm = opts?.pickAlgorithm || getPrioritizedHash
-    const keys = Object.keys(this)
-    return keys.reduce((acc, algo) => {
-      return pickAlgorithm(acc, algo) || acc
+    const keys = Object.keys(this).filter(k => {
+      if (hashes?.length) {
+        return hashes.includes(k)
+      }
+      return true
     })
+    if (keys.length) {
+      return keys.reduce((acc, algo) => pickAlgorithm(acc, algo) || acc)
+    }
+    // no intersection between this and hashes,
+    return null
   }
 }
 
@@ -550,7 +563,7 @@ function createIntegrity (opts) {
   }
 }
 
-const NODE_HASHES = new Set(crypto.getHashes())
+const NODE_HASHES = crypto.getHashes()
 
 // This is a Best Effort™ at a reasonable priority for hash algos
 const DEFAULT_PRIORITY = [
@@ -560,7 +573,7 @@ const DEFAULT_PRIORITY = [
   'sha3',
   'sha3-256', 'sha3-384', 'sha3-512',
   'sha3_256', 'sha3_384', 'sha3_512',
-].filter(algo => NODE_HASHES.has(algo))
+].filter(algo => NODE_HASHES.includes(algo))
 
 function getPrioritizedHash (algo1, algo2) {
   /* eslint-disable-next-line max-len */
diff --git a/test/match.js b/test/match.js
@@ -13,8 +13,8 @@ function hash (data, algorithm) {
 }
 
 test('hashes should match when valid', t => {
-  const sha = hash(TEST_DATA, 'sha512')
-  const integrity = `sha512-${sha}`
+  const integrity = `sha512-${hash(TEST_DATA, 'sha512')}`
+  const otherIntegrity = `sha512-${hash('mismatch', 'sha512')}`
   const parsed = ssri.parse(integrity, { single: true })
   t.same(
     parsed.match(integrity, { single: true }),
@@ -47,5 +47,10 @@ test('hashes should match when valid', t => {
     false,
     'null integrity just returns false'
   )
+  t.same(
+    parsed.match(otherIntegrity),
+    false,
+    'should not match with a totally different integrity'
+  )
   t.end()
 })
