Security issue with UserDefault and AutoFill

[eonist]

Potential problem:

https://github.com/keepassium/KeePassium/blob/master/KeePassium%20AutoFill/util/QuickAutoFillPrompt.swift

You can bypass autofill authentication by changing the UserDefault lastSeenDate value with a plist editor app.

Proposed solution:

1. Store this data in keychain. Which cannot be edited in external apps.
2. Or encrypt the userDefault

[keepassium]

Thank you for the feedback!

https://github.com/keepassium/KeePassium/blob/master/KeePassium%20AutoFill/util/QuickAutoFillPrompt.swift

Technically, this specific file handles the "Activate Quick AutoFill" announcement. Prevents it from becoming annoyingly frequent, to be precise.

But in general you are right, timeouts can be a problem. If the lock timeout is not "immediately", the attacker can adjust the recentUserActivityTimestamp to emulate recent user activity and thus postpone the lock-up. Moving the timestamp from UserDefaults to the keychain would not help much, though: someone who can modify UserDefaults on an unlocked iOS device might as well simply adjust the system time.

To handle timeouts reliably, we would need an external source of time. I guess, something like reaching out to Google/Apple servers (with certificate pinning). This would be both slow and violate the offline-first design requirement. Back in 2018, this was an immediate no-go, so I had to leave this setup as-is.

I you have any suggestions, I would be happy to hear them.

[eonist]

Sure. You encrypt the UserDefaults. There are opensource libs that do this. Just a google away. I have not looked into how you store your other userdefs, but if you store any bools for pin or bio-auth activation then these should also be in that encrypted userdef.

So when you have your encrypted userdef setup. You can store a timestamp when the user authenticate the first time with AutoFill. Then when the user try to re-authenticate after less than the timeout say 30sec. All you do is assert if that time stamp exist. and that it's within 30sec. An attacker can't store anything in the encrypted userdef with these apps that can access and edit other apps userdefs in iOS. Because the userdef is all scrambled. Only when the user is in the autofill app with the shared keychain with the correct app group entitlements can access this userdef. Also make sure the keychain for this encrypted userdef is set to a permission that requires only the device to be unlocked etc.

And you don't need to add NTPTime for this setup, as the attacker won't be able to inject a time in the first place. That's the beauty of encrypted userdefs. Something apple should have in place in the first place. 😅 so many doors are open because of this. They will probably add it soon tho.

If the attacker turns back the system clock. The attacker would have to know the window when the autofill authentication was unlocked. You could reset this timestamp after one failed try for instance. That would limit the window of opportunity for an attack. Or just require NTP time. (there are libs for this too that are OSS) I mean if you're using autofill, you're definitely online. So I dont see the offline part being a problem. If people are offline and still autofill, ok just ask for bio-auth twice instead of the more convenient 30sec grace period. I think that will never happen even if you have millions of users. There is also a notification you can listen to if the user change the system clock. I have not checked if its available for macOS but it exists for iOS. You could use this notificaion to void the timestamp.

But the absolute simplest solution would be to just store the timestamp in the keychain. Altho an encrypted UserDefault would be the more "correct" implementation and can be used for other things as well described above. Also more private and secure.

If you have more questions I'm happy to help. This shouldn't take too long to add. You can create a separate userdef for this case only. So it doesn't regress the app for older users etc.

Full disclosure. I might be missing something in this implementation. It's just an idea for a solution. Happy to brainstorm issues with the above proposal.

[keepassium]

Thank you for the detailed response!

that it's within 30sec.

That's the problem I was talking about. That "within" condition has two parameters: the timestamp and the current time. Even if we protect the former, we cannot protect the latter.

If the attacker turns back the system clock. The attacker would have to know the window when the autofill authentication was unlocked.

I agree. However, considering that the timeout can be customized from seconds to hours, the window can be quite wide. Sure, a plain-text timestamp makes the guessing unnecessary. But even otherwise, an attacker capable of examining UserDefaults would not have much trouble guessing the time window (or extracting it from system activity logs). Which makes timestamp protection pretty much useless anyway.

You could reset this timestamp after one failed try for instance.

This is already the case. If the attacker missed the database lock timeout, the DBs get closed and their keys are erased from the keychain.

Or just require NTP time. (…) I mean if you're using autofill, you're definitely online. So I dont see the offline part being a problem.

It's not about the "can" (ability to connect), it's about the "may" (user permission). Most users need KeePassium to stay offline. So normally the app must stay offline.

If people are offline and still autofill, ok just ask for bio-auth twice instead of the more convenient 30sec grace period.

Considering the must-stay-offline mentioned above, the app would default to biometrics every time. Always behaving as if the timeout was "immedaitely", thus rendering any timeout customization irrelevant.

There is also a notification you can listen to if the user change the system clock.

This is useful, thanks.

I will move the timestamp to keychain, and probably track it using system uptime instead of wall clock. But these are still cosmetic improvements, the best defence is to have zero timeout (which, incidentally, is the default setting :)

[eonist]

I think for you the best option is to:

1. If you put start and end time in SecureUserDefault. No-one will be able to insert new data externally (well they can, but it won't decrypt to useable data). Exploit window closed
2. If you listen for time travel OS notification. And void the date if this happens. Exploit window closed

With this solution. NTPTime is not a concern, offline or online is not a concern. And the Autofill has no attack vector that I can think of anymore. It's a clean solution. That I bet 1password and friends are using.

Storing things in keychain is finicky. It's better to store the priv-key of the secure UserDefault in keychain. You then also have nice place to store other user settings. Good practice anyways. But storing it in keychain is also better than not securing the UserDefault. Implementation is at your discretion.

[keepassium]

Thanks for the input, André.

If you listen for time travel OS notification. And void the date if this happens. Exploit window closed

On a closer look, this is not a very good option… Reportedly, NSSystemClockDidChangeNotification are redundant. Which means a lot of people would get locked out on a daily basis. So, yes, technically the exploit window is closed. Practically, however, the app becomes unusable, some users get locked out forever and your project sinks in negative reviews and support emails. Been there, done that, would not recommend.

If you put start and end time in SecureUserDefault.

I'm not sure, the start and end time of what exactly? Opening and closing the AutoFill? But that won't fix the untrusted clock issue…

[eonist]

Why would they get locked out? This autofill date stamp is in place for pure convenience purposes, so that the user doesn't have to re-authenticate when the AF extension asks for permission a second time. As the LAContext isn't carried over between AF extension states. The only thing that would happen is that the user would get 2, 3, 4 Biometric authentication requests instead of the more convenient 1, which is the reason this date stamping is in place. I don't think NSSystemClockDidChangeNotification is broken from apples side. I would just do some testing or find more corroborative data if this notification is useful or not. That being said, this is how I read this. I could be wrong.

And for the time stamps. You make a start time stamp when the first LAContext event is approved. Then if you set the grace period to 30secs. You check if current time is within that range. It's impossible for an attacker to insert untrusted clock time. As the SecUserDef is encrypted and won't decrypt to anything else than what's recored in the app. You invalidate the timestamp if NSSystemClockDidChangeNotification is triggered. That's how I see this being implemented.

[eonist]

Maybe lockwise already has an implementation for this. Worth a look: https://github.com/mozilla-lockwise/lockwise-ios

[keepassium]

Thanks, André, I will look into it.

[keepassium]

Well, it sure took some time :) Closed via b1057a1adb9b76e02d5cfac6020229043ad906f5

[eonist]

Hehe. Nice! 🎉 it was never critical, you have to have local access so hard to exploit. hopefully the keychain value migration goes smoothly for everyone 💪 maybe do phased release to make sure nothing was missed etc. ✌️ have gd weekend!

[keepassium]

hopefully the keychain value migration goes smoothly for everyone 💪

Yeah, a lot of experience in these words :)

maybe do phased release to make sure nothing was missed etc. ✌️

Always do. After some beta testing, too.