unjs / ohash

Super fast hashing library based on murmurhash3 written in Vanilla JS
MIT License
553 stars 11 forks source link

Invalid Base64 Implementation #85

Open Whipstickgostop opened 1 day ago

Whipstickgostop commented 1 day ago

Environment

Impacting all versions of ohash and node (since base64/sha256 were added)

Reproduction

https://jsfiddle.net/5wvt3qdx/

import * as ohash from "https://esm.run/ohash"

function hexToBase64(hexstring) {
  return btoa(
    hexstring
      .match(/\w{2}/g)
      .map(function (a) {
        return String.fromCharCode(parseInt(a, 16))
      })
      .join(""),
  )
}

function hexToByteString(hex) {
  let s = []
  for (let c = 0; c < hex.length; c += 2) s.push(hex.substr(c, 2).toUpperCase())
  return s.join("-")
}

const testData = {
  password: "streamer.bot",
  salt: "yEcA0VWTHfWyTsBfaL9HNXmDS58bJqNhXVSm+TDzcfQ=",
  challenge: "lSZ7S+QlkePk4ad0nyn5VdsD8UECKaxJaHgg/GJwPy4=",
  authentication: "bvTLo6kaoXJcvhf9LN97LrAYVlOvtYpDlwYJQzsl60E=",
}

let secret = testData.password + testData.salt

// this method works correctly
let secretSha256 = ohash.sha256(secret)

// this method produces an unexpected result, missing base64 chars, +, /, =
let secretBase64Ohash = ohash.sha256base64(secret)

let secretBase64 = hexToBase64(secretSha256)

let auth = secretBase64 + testData.challenge

// this method works correctly
let authSha256 = ohash.sha256(auth)

// this method produces an unexpected result, missing base64 chars, +, /, =
let authBase64Ohash = ohash.sha256base64(auth)

let authBase64 = hexToBase64(authSha256)

console.log("Secret:", secret)
console.log("Secret SHA256:", hexToByteString(secretSha256))
console.log("Secret Base64:", secretBase64)
console.log("Secret Base64 (ohash)", secretBase64Ohash)
console.log("Auth:", auth)
console.log("Auth SHA256:", hexToByteString(authSha256))
console.log("Auth Base64:", authBase64)
console.log("Auth Base64 (ohash)", authBase64Ohash)
"Secret:", "streamer.botyEcA0VWTHfWyTsBfaL9HNXmDS58bJqNhXVSm+TDzcfQ="
"Secret SHA256:", "3D-4B-42-C7-D0-B2-DE-82-DC-87-FC-83-3E-2B-EC-70-07-3E-4B-23-CC-D5-E0-89-2A-46-4D-53-3D-AA-D5-57"
"Secret Base64:", "PUtCx9Cy3oLch/yDPivscAc+SyPM1eCJKkZNUz2q1Vc="
"Secret Base64 (ohash)", "PUtCx9Cy3oLchyDPivscAcSyPM1eCJKkZNUz2q1Vc"

"Auth:", "PUtCx9Cy3oLch/yDPivscAc+SyPM1eCJKkZNUz2q1Vc=lSZ7S+QlkePk4ad0nyn5VdsD8UECKaxJaHgg/GJwPy4="
"Auth SHA256:", "6E-F4-CB-A3-A9-1A-A1-72-5C-BE-17-FD-2C-DF-7B-2E-B0-18-56-53-AF-B5-8A-43-97-06-09-43-3B-25-EB-41"
"Auth Base64:", "bvTLo6kaoXJcvhf9LN97LrAYVlOvtYpDlwYJQzsl60E="
"Auth Base64 (ohash)", "bvTLo6kaoXJcvhf9LN97LrAYVlOvtYpDlwYJQzsl60E"

Describe the bug

Base64 character map is missing +, /, and padding =

https://github.com/unjs/ohash/commit/778413f5a848fb310dd08b17bbe65f2564ba2222#diff-6831e97cd83e9338eef1f644054e19e29bfc4c9898f875ce3671575ef099592cR71

This currently breaks all comparisons of ohash sha256base64 vs. any externally generated sha256 base64 digest.

Additional context

This can be easily resolved by adding + and / to the line referenced in the link above, and then also adding the base64 padding char, = to the resulting string.

const keyStr = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'

This change will also impact existing test cases.

Logs

No response

pi0 commented 22 hours ago

Thanks for reporting issue. I can take naming can be misleading indeed. We use base64 (instead of default hex) to fit more bytes in less string chars mainly and drop special chars to make hash more compatible.

We should improve docs.

I think if you need a version that allows two way conversion (between hex and base64) and compatible externally, we could add an option.

Whipstickgostop commented 14 hours ago

The resulting string is not base64 if the 62nd and 63rd characters are being stripped entirely:

RFC 4648 - Base 64 Encoding

Sadly, I believe returning a non-compliant base64 string from a base64 function is a larger issue than simply documentation, but nonetheless, improved docs would certainly save some confusion 😄