search

freeotp / freeotp-android

Apache License 2.0
1.44k stars 303 forks source link

Parse leading/trailing whitespace leniently #364

Open dlenski opened 1 year ago

dlenski commented 1 year ago

I'm the maintainer of python-vipaccess, which can provision Symantec/Broadcom VIPAccess soft-tokens and convert them to otpauth:// URIs, e.g.:

$ vipaccess provision -p
Generating request...
Fetching provisioning response from Symantec server...
Getting token from response...
Decrypting token...
Checking token against Symantec server...
Credential created successfully:
    otpauth://totp/VIP%20Access:SYMC59691497?secret=I7GNMD6BC56SV563Q6DEIMGVFE3OQBBP&digits=6&algorithm=SHA1&image=https%3A%2F%2Fraw.githubusercontent.com%2Fdlenski%2Fpython-vipaccess%2Fmaster%2Fvipaccess.png&issuer=VIP+Access&period=30
This credential expires on this date: 2026-09-12T16:51:43.643Z

You will need the ID to register this credential: SYMC59691497

You can use oathtool to generate the same OTP codes
as would be produced by the official VIP Access apps:

    oathtool    -b --totp I7GNMD6BC56SV563Q6DEIMGVFE3OQBBP  # output one code
    oathtool -v -b --totp I7GNMD6BC56SV563Q6DEIMGVFE3OQBBP  # ... with extra information

Until recently, I was often using FreeOTP 1.x to sanity-check the generated otpauth:// URIs. When I recently, finally upgraded to FreeOTP 2.x, I was surprised to discover that QR codes generated as follows seemed not to work in FreeOTP…

$ echo "otpauth://…" | qrencode -t utf8
<qr code>

I'd get the dreaded "Token is invalid!" method when scanning this.

I'd played around with a bunch of changes to the URI parameters, dug around in some of the recent issues (e.g. #295), and eventually discovered that it was failing because of trailing whitespace (echo, not echo -n). With a little more testing, I discovered that FreeOTP 2.x refuses to parse any QR codes containing trailing or leading whitespace of any kind.

Ask: modify the parser to strip leading/trailing whitespace (e.g. [ \t\r\n]) from otpauth:// URIs.

(Many other Android barcode scanning apps already do this automatically.)

justin-stephenson commented 1 year ago

Thank you for reporting this, I am okay to strip out non-URL encoded whitespace characters. PRs are welcome.

justin-stephenson commented 1 year ago

Ask: modify the parser to strip leading/trailing whitespace (e.g. [ \t\r\n]) from otpauth:// URIs.

(Many other Android barcode scanning apps already do this automatically.)

Do you propose this to be performed by the QR code library we are using? https://github.com/zxing/zxing