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

Feature request: Store session cookies in secure storage and not in localstorage #194

Open
BorntraegerMarc opened this issue Mar 20, 2019 · 9 comments
Labels

Comments

@BorntraegerMarc
Copy link

The local storage is very unsecure. Thus we shouldn't store cookies (or at least session cookies) in it because it's open for potential attackers.

I understand that risk of an attack is reduced since we are talking about a WebView inside a native app. But still when some developer navigates within the WebView to a different website there might be a scenario where the local storage can get exploited.

@silkimen
Copy link
Owner

Hi Marc, I understand your concerns about security. I also think that the risk is reduced in our special case, because it's running in a sand-boxed app which can't be inspected with developer tools when running in production mode (at least on a device which is not rooted / jail-broken). But it still feels wrong, so I'll put this on the road map. 👍

@silkimen
Copy link
Owner

Hi Marc, I've done some research for this feature request. The problem with secure storage on devices is that they all use the device pin, passphrase or lock code to encrypt data. You can't use the secure storage without having a lock code defined. Therefore I don't see a better option to save the data other than in local storage.
If the developer decides to navigate to another website within the WebView it's in another context and the local storage is bound to your context, so this website won't have access to the cookie store and nothing bad can happen.

@BorntraegerMarc
Copy link
Author

Thanks for getting back with this research.

The problem with secure storage on devices is that they all use the device pin, passphrase or lock code to encrypt data. You can't use the secure storage without having a lock code defined.

Indeed. We work-around this limitation in our app by only using secure storage when it's available. On other devices the "normal" storage is used. We follow the practice that if the user doesn't have a pin setup he probably doesn't care so much about security.

I guess malicious JS can still be injected in a WebView, even though it's sandboxed? Especially if a remote http website (without https) is being loaded.

My counter proposal would be to take advantage of the secure storage whenever available and if it's not possible to fallback to local storage / native "normal" storage.

@BorntraegerMarc
Copy link
Author

BorntraegerMarc commented Jan 13, 2020

another side effect of not storing cookies in native storage (secure or "normal") is that when I updated ionic web view the URL scheme changed from http://localhost:8080 to http://ionic on iOS. This led to the local storage values not existing anymore after updating.

If this plugin would keep track of the values in the native storage then it would be unaffected from webview changes.

@gaspachoandalus
Copy link

Hi,
What about letting the user choose where he wants to store the cookies ?

I'm currently adding the following interface to cordova.plugin.http to be able provide a custom storage medium:

/**
 * Provides a custom cookie storage, to override the default localStorage
 * @param {Object} storageImpl 
 * @param {(name: string, value: string) => void} storageImpl.setItem 
 * @param {(name: string) => string} storageImpl.getItem 
 * @param {(name: string) => void} storageImpl.removeItem 
 */
function setCookieStorageImpl(storageImpl)

In my case, I can use my own implementation which encrypts and stores data in the native storage, but anyone could use it to store cookies in the native storage for example.

@silkimen
Copy link
Owner

silkimen commented Oct 7, 2021

@gaspachoandalus Yes, this sounds good actually. But your JSDoc comments are confusing me a bit. You are documenting four params, but it's actually only one parameter in the function signature. I'd propose, just open a PR and we can discuss it there.

@Lindsor
Copy link

Lindsor commented Oct 7, 2021

Hi, What about letting the user choose where he wants to store the cookies ?

I'm currently adding the following interface to cordova.plugin.http to be able provide a custom storage medium:

/**
 * Provides a custom cookie storage, to override the default localStorage
 * @param {Object} storageImpl 
 * @param {(name: string, value: string) => void} storageImpl.setItem 
 * @param {(name: string) => string} storageImpl.getItem 
 * @param {(name: string) => void} storageImpl.removeItem 
 */
function setCookieStorageImpl(storageImpl)

In my case, I can use my own implementation which encrypts and stores data in the native storage, but anyone could use it to store cookies in the native storage for example.

I would suggest the storageImpl functions return Promise wrapped versions of your return types though.
Storage access is almost always asynchronous in nature.

@gaspachoandalus
Copy link

@silkimen sorry for the confusion, there might be multiple ways to declare objects in JSDoc.
Here's how VSCode displays this doc:
image

I created PR #434

@gaspachoandalus
Copy link

@Lindsor yes, it would be better to have an asynchronous implementation. I have not checked how many changes would be required though.
This solution was the easiest since it really works like localStorage implementation (which is synchronous).

My real storage implementation:

  • reads the cookie from native storage at application startup and stores it in memory
  • setItem and removeItem stores the cookie in memory synchronously, and writes it to native storage asynchronously
  • getItem reads from memory synchronously

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants