buttercup / buttercup-core

:tophat: The mighty NodeJS password vault
http://buttercup.pw/
MIT License
472 stars 57 forks source link

OTP url parser does not like spaces #278

Open Kyu opened 4 years ago

Kyu commented 4 years ago

So I had a OTP Url like so otpauth://totp/Slack ({workspace_name}):{email}?secret={secret_code}&issuer=Slack
the space was inserted as %20

When trying to import it via Barcode, it would buzz(on the iOS app) and do nothing, so I opened the barcode in a reader and tried to manually enter it in the Buttercup app. Saving would cause it to crash. I eventually saved it as just the {secret_code} for future usage, but then I opened my vault in the Chrome extension and changed the OTP url to remove the ({workspace_name}) and space before it, and it worked just fine. The new URL looked like otpauth://totp/Slack:{email}?secret={secret_code}&issuer=Slack

TL;DR I think the space caused some issue in the parser, so that's basically what I'm reporting here, the OTP url parser does not like spaces.

perry-mitchell commented 4 years ago

I've checked this with a lot of online OTP URI generators, and they all insert %20 as expected:

image

(this was taken from here)

URIs shouldn't support spaces anyway, as they should always be encoded to %20 (or + I guess in some cases). Considering this, I'm starting to think that this issue might be invalid. Any systems generating an OTP URI with spaces are probably doing it erroneously.

Kyu commented 4 years ago

That makes sense, I'll see if I can take the issue up with slack then

perry-mitchell commented 4 years ago

@Kyu Let us know how you go!

It's probably not a lot of work implementing a function to replace spaces with %20, but this feels hacky. I'm at least open to hearing other opinions, as well.