search

freeotp / freeotp-android

Apache License 2.0
1.43k stars 300 forks source link

Sanitize or validate input strings for special characters such as newline #73

Open npmccallum opened 8 years ago

npmccallum commented 8 years ago

Reported by nakato on 19 May 2016 03:28 UTC java.lang.NullPointerException: Attempt to invoke virtual method 'int org.fedorahosted.freeotp.Token.getDigits()' on a null object reference

Producing the bug:

I have a populated FreeOTP and it works after the process is closed or the phone is rebooted. After I add the otpauth for my DigitalOcean account the application gives me the OTP codes and I'm able to use it as normal for authentication. Short app switches between FreeOTP and other apps work.

Now after some time passes with the app in the background, or a reboot of the phone, when I attempt to start the application it immediately crashes.

If I dump the storage, erasing all my OTP details, the app will start again and I can re-import the OTP accounts, everything will work normal, until the DigitalOcean account is added again.

Let me know what I can do to help track down this bug.

Cheers

npmccallum commented 8 years ago

Comment by nakato on 19 May 2016 10:26 UTC I am wrong, any new OTP key seems to crash the application on load.

I took a wild stab in the dark and got myself a debuggable set binary and fiddled with tokens.xml, knowing gson.fromJson was loading the token objects, and seem to have got a hit.

A censored tokens.xml file:

<?xml version='1.0' encoding='utf-8' standalone='yes' ?>

{"algo":"SHA1","counter":0,"digits":6,"issuerExt":"DigitalOcean","issuerInt":"DigitalOcean\n","label":"USERNAME","period":30,"secret":[ ["DigitalOcean\n:USERNAME"](0,0,0,0,0,0,0,0,0,0],"type":"TOTP"}

The crash no longer occurs if the '\n' is taken out of the 'tokenOrder' list. Also of note is that issuerInt has a '\n' as well.

A QRCode was used for import that resolves to, again censored:

otpauth://totp/DigitalOcean:USERNAME?secret=SECRET&issuer=DigitalOcean

npmccallum commented 8 years ago

Comment by nakato on 19 May 2016 10:35 UTC Ah, this comes down to how I'm generating my QR codes out of backups, they end up with a '\n' at the end of them.

If I make sure my URL does not contain a \n I don't reproduce the bug.

Seems like something that would be good to guard against, but regardless, sorry for the noise.