Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid Base64 Implementation #85

Open
Whipstickgostop opened this issue Oct 12, 2024 · 2 comments
Open

Invalid Base64 Implementation #85

Whipstickgostop opened this issue Oct 12, 2024 · 2 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@Whipstickgostop
Copy link

Whipstickgostop commented Oct 12, 2024

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 =

778413f#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 Whipstickgostop added the bug Something isn't working label Oct 12, 2024
@pi0
Copy link
Member

pi0 commented Oct 13, 2024

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 pi0 added documentation Improvements or additions to documentation enhancement New feature or request and removed bug Something isn't working labels Oct 13, 2024
@Whipstickgostop
Copy link
Author

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 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants