Merge pull request from GHSA-9qxr-qj54-h672

Co-authored-by: uzlopak <aras.abbasi@googlemail.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>

Author: mcollina

benchmarks/fetch/bytes-match.mjs

@@ -0,0 +1,24 @@
+import { createHash } from 'node:crypto'
+import { bench, run } from 'mitata'
+import { bytesMatch } from '../../lib/web/fetch/util.js'
+
+const body = Buffer.from('Hello world!')
+const validSha256Base64 = `sha256-${createHash('sha256').update(body).digest('base64')}`
+const invalidSha256Base64 = `sha256-${createHash('sha256').update(body).digest('base64')}`
+const validSha256Base64Url = `sha256-${createHash('sha256').update(body).digest('base64url')}`
+const invalidSha256Base64Url = `sha256-${createHash('sha256').update(body).digest('base64url')}`
+
+bench('bytesMatch valid sha256 and base64', () => {
+  bytesMatch(body, validSha256Base64)
+})
+bench('bytesMatch invalid sha256 and base64', () => {
+  bytesMatch(body, invalidSha256Base64)
+})
+bench('bytesMatch valid sha256 and base64url', () => {
+  bytesMatch(body, validSha256Base64Url)
+})
+bench('bytesMatch invalid sha256 and base64url', () => {
+  bytesMatch(body, invalidSha256Base64Url)
+})
+
+await run()

lib/fetch/util.js

@@ -7,14 +7,18 @@ const { isBlobLike, toUSVString, ReadableStreamFrom } = require('../core/util')
 const assert = require('assert')
 const { isUint8Array } = require('util/types')
 
+let supportedHashes = []
+
 // https://nodejs.org/api/crypto.html#determining-if-crypto-support-is-unavailable
 /** @type {import('crypto')|undefined} */
 let crypto
 
 try {
   crypto = require('crypto')
+  const possibleRelevantHashes = ['sha256', 'sha384', 'sha512']
+  supportedHashes = crypto.getHashes().filter((hash) => possibleRelevantHashes.includes(hash))
+/* c8 ignore next 3 */
 } catch {
-
 }
 
 function responseURL (response) {
@@ -542,66 +546,56 @@ function bytesMatch (bytes, metadataList) {
     return true
   }
 
-  // 3. If parsedMetadata is the empty set, return true.
+  // 3. If response is not eligible for integrity validation, return false.
+  // TODO
+
+  // 4. If parsedMetadata is the empty set, return true.
   if (parsedMetadata.length === 0) {
     return true
   }
 
-  // 4. Let metadata be the result of getting the strongest
+  // 5. Let metadata be the result of getting the strongest
   //    metadata from parsedMetadata.
-  const list = parsedMetadata.sort((c, d) => d.algo.localeCompare(c.algo))
-  // get the strongest algorithm
-  const strongest = list[0].algo
-  // get all entries that use the strongest algorithm; ignore weaker
-  const metadata = list.filter((item) => item.algo === strongest)
+  const strongest = getStrongestMetadata(parsedMetadata)
+  const metadata = filterMetadataListByAlgorithm(parsedMetadata, strongest)
 
-  // 5. For each item in metadata:
+  // 6. For each item in metadata:
   for (const item of metadata) {
     // 1. Let algorithm be the alg component of item.
     const algorithm = item.algo
 
     // 2. Let expectedValue be the val component of item.
-    let expectedValue = item.hash
+    const expectedValue = item.hash
 
     // See https://github.com/web-platform-tests/wpt/commit/e4c5cc7a5e48093220528dfdd1c4012dc3837a0e
     // "be liberal with padding". This is annoying, and it's not even in the spec.
 
-    if (expectedValue.endsWith('==')) {
-      expectedValue = expectedValue.slice(0, -2)
-    }
-
     // 3. Let actualValue be the result of applying algorithm to bytes.
     let actualValue = crypto.createHash(algorithm).update(bytes).digest('base64')
 
-    if (actualValue.endsWith('==')) {
-      actualValue = actualValue.slice(0, -2)
+    if (actualValue[actualValue.length - 1] === '=') {
+      if (actualValue[actualValue.length - 2] === '=') {
+        actualValue = actualValue.slice(0, -2)
+      } else {
+        actualValue = actualValue.slice(0, -1)
+      }
     }
 
     // 4. If actualValue is a case-sensitive match for expectedValue,
     //    return true.
-    if (actualValue === expectedValue) {
-      return true
-    }
-
-    let actualBase64URL = crypto.createHash(algorithm).update(bytes).digest('base64url')
-
-    if (actualBase64URL.endsWith('==')) {
-      actualBase64URL = actualBase64URL.slice(0, -2)
-    }
-
-    if (actualBase64URL === expectedValue) {
+    if (compareBase64Mixed(actualValue, expectedValue)) {
       return true
     }
   }
 
-  // 6. Return false.
+  // 7. Return false.
   return false
 }
 
 // https://w3c.github.io/webappsec-subresource-integrity/#grammardef-hash-with-options
 // https://www.w3.org/TR/CSP2/#source-list-syntax
 // https://www.rfc-editor.org/rfc/rfc5234#appendix-B.1
-const parseHashWithOptions = /((?<algo>sha256|sha384|sha512)-(?<hash>[A-z0-9+/]{1}.*={0,2}))( +[\x21-\x7e]?)?/i
+const parseHashWithOptions = /(?<algo>sha256|sha384|sha512)-((?<hash>[A-Za-z0-9+/]+|[A-Za-z0-9_-]+)={0,2}(?:\s|$)( +[!-~]*)?)?/i
 
 /**
  * @see https://w3c.github.io/webappsec-subresource-integrity/#parse-metadata
@@ -615,8 +609,6 @@ function parseMetadata (metadata) {
   // 2. Let empty be equal to true.
   let empty = true
 
-  const supportedHashes = crypto.getHashes()
-
   // 3. For each token returned by splitting metadata on spaces:
   for (const token of metadata.split(' ')) {
     // 1. Set empty to false.
@@ -626,7 +618,11 @@ function parseMetadata (metadata) {
     const parsedToken = parseHashWithOptions.exec(token)
 
     // 3. If token does not parse, continue to the next token.
-    if (parsedToken === null || parsedToken.groups === undefined) {
+    if (
+      parsedToken === null ||
+      parsedToken.groups === undefined ||
+      parsedToken.groups.algo === undefined
+    ) {
       // Note: Chromium blocks the request at this point, but Firefox
       // gives a warning that an invalid integrity was given. The
       // correct behavior is to ignore these, and subsequently not
@@ -635,11 +631,11 @@ function parseMetadata (metadata) {
     }
 
     // 4. Let algorithm be the hash-algo component of token.
-    const algorithm = parsedToken.groups.algo
+    const algorithm = parsedToken.groups.algo.toLowerCase()
 
     // 5. If algorithm is a hash function recognized by the user
     //    agent, add the parsed token to result.
-    if (supportedHashes.includes(algorithm.toLowerCase())) {
+    if (supportedHashes.includes(algorithm)) {
       result.push(parsedToken.groups)
     }
   }
@@ -652,6 +648,82 @@ function parseMetadata (metadata) {
   return result
 }
 
+/**
+ * @param {{ algo: 'sha256' | 'sha384' | 'sha512' }[]} metadataList
+ */
+function getStrongestMetadata (metadataList) {
+  // Let algorithm be the algo component of the first item in metadataList.
+  // Can be sha256
+  let algorithm = metadataList[0].algo
+  // If the algorithm is sha512, then it is the strongest
+  // and we can return immediately
+  if (algorithm[3] === '5') {
+    return algorithm
+  }
+
+  for (let i = 1; i < metadataList.length; ++i) {
+    const metadata = metadataList[i]
+    // If the algorithm is sha512, then it is the strongest
+    // and we can break the loop immediately
+    if (metadata.algo[3] === '5') {
+      algorithm = 'sha512'
+      break
+    // If the algorithm is sha384, then a potential sha256 or sha384 is ignored
+    } else if (algorithm[3] === '3') {
+      continue
+    // algorithm is sha256, check if algorithm is sha384 and if so, set it as
+    // the strongest
+    } else if (metadata.algo[3] === '3') {
+      algorithm = 'sha384'
+    }
+  }
+  return algorithm
+}
+
+function filterMetadataListByAlgorithm (metadataList, algorithm) {
+  if (metadataList.length === 1) {
+    return metadataList
+  }
+
+  let pos = 0
+  for (let i = 0; i < metadataList.length; ++i) {
+    if (metadataList[i].algo === algorithm) {
+      metadataList[pos++] = metadataList[i]
+    }
+  }
+
+  metadataList.length = pos
+
+  return metadataList
+}
+
+/**
+ * Compares two base64 strings, allowing for base64url
+ * in the second string.
+ *
+* @param {string} actualValue always base64
+ * @param {string} expectedValue base64 or base64url
+ * @returns {boolean}
+ */
+function compareBase64Mixed (actualValue, expectedValue) {
+  if (actualValue.length !== expectedValue.length) {
+    return false
+  }
+  for (let i = 0; i < actualValue.length; ++i) {
+    if (actualValue[i] !== expectedValue[i]) {
+      if (
+        (actualValue[i] === '+' && expectedValue[i] === '-') ||
+        (actualValue[i] === '/' && expectedValue[i] === '_')
+      ) {
+        continue
+      }
+      return false
+    }
+  }
+
+  return true
+}
+
 // https://w3c.github.io/webappsec-upgrade-insecure-requests/#upgrade-request
 function tryUpgradeRequestToAPotentiallyTrustworthyURL (request) {
   // TODO
@@ -1067,5 +1139,6 @@ module.exports = {
   urlHasHttpsScheme,
   urlIsHttpHttpsScheme,
   readAllBytes,
-  normalizeMethodRecord
+  normalizeMethodRecord,
+  parseMetadata
 }

test/fetch/integrity.js

@@ -4,6 +4,16 @@ const { test } = require('tap')
 const { createServer } = require('http')
 const { createHash, getHashes } = require('crypto')
 const { gzipSync } = require('zlib')
+/*
+=======
+const { test, after } = require('node:test')
+const { tspl } = require('@matteo.collina/tspl')
+const assert = require('node:assert')
+const { createServer } = require('node:http')
+const { createHash, getHashes } = require('node:crypto')
+const { gzipSync } = require('node:zlib')
+>>>>>>> d542b8cd (Merge pull request from GHSA-9qxr-qj54-h672)
+*/
 const { fetch, setGlobalDispatcher, Agent } = require('../..')
 const { once } = require('events')
 
@@ -148,3 +158,190 @@ test('request with sha512 hash', { skip: !supportedHashes.includes('sha512') },
     integrity: 'sha512-ypeBEsobvcr6wjGzmiPcTaeG7/gUfE5yuYB3ha/uSLs='
   }))
 })
+
+test('request with correct integrity checksum (base64url)', (t) => {
+  t.plan(1)
+  const body = 'Hello world!'
+  const hash = createHash('sha256').update(body).digest('base64url')
+
+  const server = createServer((req, res) => {
+    res.end(body)
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, async () => {
+    const response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha256-${hash}`
+    })
+    t.equal(body, await response.text())
+  })
+})
+
+test('request with incorrect integrity checksum (base64url)', (t) => {
+  t.plan(1)
+
+  const body = 'Hello world!'
+  const hash = createHash('sha256').update('invalid').digest('base64url')
+
+  const server = createServer((req, res) => {
+    res.end(body)
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, async () => {
+    await t.rejects(fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha256-${hash}`
+    }))
+  })
+})
+
+test('request with incorrect integrity checksum (only dash)', (t) => {
+  t.plan(1)
+
+  const body = 'Hello world!'
+
+  const server = createServer((req, res) => {
+    res.end(body)
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, async () => {
+    await t.rejects(fetch(`http://localhost:${server.address().port}`, {
+      integrity: 'sha256--'
+    }))
+  })
+})
+
+test('request with incorrect integrity checksum (non-ascii character)', (t) => {
+  t.plan(1)
+
+  const body = 'Hello world!'
+
+  const server = createServer((req, res) => {
+    res.end(body)
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, async () => {
+    await t.rejects(() => fetch(`http://localhost:${server.address().port}`, {
+      integrity: 'sha256-ä'
+    }))
+  })
+})
+
+test('request with incorrect stronger integrity checksum (non-ascii character)', (t) => {
+  t.plan(2)
+
+  const body = 'Hello world!'
+  const sha256 = createHash('sha256').update(body).digest('base64')
+  const sha384 = 'ä'
+
+  const server = createServer((req, res) => {
+    res.end(body)
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, async () => {
+    await t.rejects(() => fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha256-${sha256} sha384-${sha384}`
+    }))
+    await t.rejects(() => fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha384-${sha384} sha256-${sha256}`
+    }))
+  })
+})
+
+test('request with correct integrity checksum (base64). mixed', (t) => {
+  t.plan(6)
+
+  const body = 'Hello world!'
+  const sha256 = createHash('sha256').update(body).digest('base64')
+  const sha384 = createHash('sha384').update(body).digest('base64')
+  const sha512 = createHash('sha512').update(body).digest('base64')
+
+  const server = createServer((req, res) => {
+    res.end(body)
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, async () => {
+    let response
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha256-${sha256} sha512-${sha512}`
+    })
+    t.equal(body, await response.text())
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha512-${sha512} sha256-${sha256}`
+    })
+
+    t.equal(body, await response.text())
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha384-${sha384} sha512-${sha512}`
+    })
+    t.equal(body, await response.text())
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha384-${sha384} sha512-${sha512}`
+    })
+    t.equal(body, await response.text())
+
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha256-${sha256} sha384-${sha384}`
+    })
+    t.equal(body, await response.text())
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha384-${sha384} sha256-${sha256}`
+    })
+    t.equal(body, await response.text())
+  })
+})
+
+test('request with correct integrity checksum (base64url). mixed', (t) => {
+  t.plan(6)
+
+  const body = 'Hello world!'
+  const sha256 = createHash('sha256').update(body).digest('base64url')
+  const sha384 = createHash('sha384').update(body).digest('base64url')
+  const sha512 = createHash('sha512').update(body).digest('base64url')
+
+  const server = createServer((req, res) => {
+    res.end(body)
+  })
+
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, async () => {
+    let response
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha256-${sha256} sha512-${sha512}`
+    })
+    t.equal(body, await response.text())
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha512-${sha512} sha256-${sha256}`
+    })
+
+    t.equal(body, await response.text())
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha384-${sha384} sha512-${sha512}`
+    })
+    t.equal(body, await response.text())
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha384-${sha384} sha512-${sha512}`
+    })
+    t.equal(body, await response.text())
+
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha256-${sha256} sha384-${sha384}`
+    })
+    t.equal(body, await response.text())
+    response = await fetch(`http://localhost:${server.address().port}`, {
+      integrity: `sha384-${sha384} sha256-${sha256}`
+    })
+    t.equal(body, await response.text())
+  })
+})

test/fetch/util.js

@@ -5,6 +5,7 @@ const { test } = t
 
 const util = require('../../lib/fetch/util')
 const { HeadersList } = require('../../lib/fetch/headers')
+const { createHash } = require('crypto')
 
 test('responseURL', (t) => {
   t.plan(2)
@@ -279,3 +280,75 @@ test('setRequestReferrerPolicyOnRedirect', nested => {
     t.equal(request.referrerPolicy, initial)
   })
 })
+
+test('parseMetadata', (t) => {
+  t.test('should parse valid metadata with option', (t) => {
+    const body = 'Hello world!'
+    const hash256 = createHash('sha256').update(body).digest('base64')
+    const hash384 = createHash('sha384').update(body).digest('base64')
+    const hash512 = createHash('sha512').update(body).digest('base64')
+
+    const validMetadata = `sha256-${hash256} !@ sha384-${hash384} !@ sha512-${hash512} !@`
+    const result = util.parseMetadata(validMetadata)
+
+    t.same(result, [
+      { algo: 'sha256', hash: hash256.replace(/=/g, '') },
+      { algo: 'sha384', hash: hash384.replace(/=/g, '') },
+      { algo: 'sha512', hash: hash512.replace(/=/g, '') }
+    ])
+    t.end()
+  })
+
+  t.test('should parse valid metadata with non ASCII chars option', (t) => {
+    const body = 'Hello world!'
+    const hash256 = createHash('sha256').update(body).digest('base64')
+    const hash384 = createHash('sha384').update(body).digest('base64')
+    const hash512 = createHash('sha512').update(body).digest('base64')
+
+    const validMetadata = `sha256-${hash256} !© sha384-${hash384} !€ sha512-${hash512} !µ`
+    const result = util.parseMetadata(validMetadata)
+
+    t.same(result, [
+      { algo: 'sha256', hash: hash256.replace(/=/g, '') },
+      { algo: 'sha384', hash: hash384.replace(/=/g, '') },
+      { algo: 'sha512', hash: hash512.replace(/=/g, '') }
+    ])
+    t.end()
+  })
+
+  t.test('should parse valid metadata without option', (t) => {
+    const body = 'Hello world!'
+    const hash256 = createHash('sha256').update(body).digest('base64')
+    const hash384 = createHash('sha384').update(body).digest('base64')
+    const hash512 = createHash('sha512').update(body).digest('base64')
+
+    const validMetadata = `sha256-${hash256} sha384-${hash384} sha512-${hash512}`
+    const result = util.parseMetadata(validMetadata)
+
+    t.same(result, [
+      { algo: 'sha256', hash: hash256.replace(/=/g, '') },
+      { algo: 'sha384', hash: hash384.replace(/=/g, '') },
+      { algo: 'sha512', hash: hash512.replace(/=/g, '') }
+    ])
+    t.end()
+  })
+
+  t.test('should set hash as undefined when invalid base64 chars are provided', (t) => {
+    const body = 'Hello world!'
+    const hash256 = createHash('sha256').update(body).digest('base64')
+    const invalidHash384 = 'zifp5hE1Xl5LQQqQz[]Bq/iaq9Wb6jVb//T7EfTmbXD2aEP5c2ZdJr9YTDfcTE1ZH+'
+    const hash512 = createHash('sha512').update(body).digest('base64')
+
+    const validMetadata = `sha256-${hash256} sha384-${invalidHash384} sha512-${hash512}`
+    const result = util.parseMetadata(validMetadata)
+
+    t.same(result, [
+      { algo: 'sha256', hash: hash256.replace(/=/g, '') },
+      { algo: 'sha384', hash: undefined },
+      { algo: 'sha512', hash: hash512.replace(/=/g, '') }
+    ])
+    t.end()
+  })
+
+  t.end()
+})