ohash icon indicating copy to clipboard operation
ohash copied to clipboard

Invalid Base64 Implementation

Open Whipstickgostop opened this issue 1 year ago • 2 comments

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

Whipstickgostop avatar Oct 12 '24 20:10 Whipstickgostop

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.

pi0 avatar Oct 13 '24 07:10 pi0

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 😄

Whipstickgostop avatar Oct 13 '24 15:10 Whipstickgostop

#106 will convert confusing util names to stringDigest with clear description of what it does in docs.

pi0 avatar Feb 19 '25 00:02 pi0

Also #111 to finally be back on standard base64 url.

pi0 avatar Feb 19 '25 10:02 pi0