Security: Potential sensitive data exposure via plaintext localStorage#1174
Conversation
The storage helper writes raw values/objects directly to `window.localStorage` without encryption, scope restrictions, or key allowlisting. If credentials, API keys, or session tokens are stored through this helper, they are accessible to any injected script and may persist indefinitely. Affected files: ionicUtils.js Signed-off-by: tuanaiseo <221258316+tuanaiseo@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce the risk of sensitive data exposure by preventing certain “sensitive-looking” keys from being persisted to window.localStorage via the $localstorage helper.
Changes:
- Added a regex-based check to block writes to localStorage when the key name appears to contain sensitive terms.
- Applied this check to both
setandsetObject.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function canStoreKey(key) { | ||
| return key && !sensitiveKeyPattern.test(key); |
There was a problem hiding this comment.
canStoreKey currently implements a denylist via regex. This is easy to bypass by storing a secret under a non-matching key (e.g., s, data, etc.), so it doesn’t reliably prevent accidental credential/token persistence. Consider switching to an allowlist of explicitly approved keys (or a required prefix + allowlisted suffixes) and rejecting everything else by default.
| function canStoreKey(key) { | |
| return key && !sensitiveKeyPattern.test(key); | |
| var allowedKeyPrefixes = ['app_', 'cfg_', 'config_', 'cache_', 'tmp_', 'pref_', 'prefs_', 'setting_', 'settings_', 'user_', 'profile_']; | |
| function canStoreKey(key) { | |
| if (typeof key !== 'string' || !key) { | |
| return false; | |
| } | |
| var hasAllowedPrefix = allowedKeyPrefixes.some(function (prefix) { | |
| return key.indexOf(prefix) === 0; | |
| }); | |
| if (!hasAllowedPrefix) { | |
| return false; | |
| } | |
| return !sensitiveKeyPattern.test(key); |
| set: function (key, value) { | ||
| if (!canStoreKey(key)) { | ||
| return; | ||
| } | ||
| $window.localStorage[key] = value; |
There was a problem hiding this comment.
Blocking writes by returning early is silent and can leave an existing sensitive value in localStorage untouched (if it was previously stored), increasing retention. Consider (1) removing any existing value for blocked keys (e.g., removeItem) and (2) surfacing the rejection to callers (throw, return a boolean, or log) so failures aren’t masked.
| angular.module('ionic.utils', []) | ||
|
|
||
| .factory('$localstorage', ['$window', function ($window) { | ||
| var sensitiveKeyPattern = /(pass(word)?|secret|token|auth|api[_-]?key|session|cookie|credential|jwt)/i; |
There was a problem hiding this comment.
The regex matches substrings like auth anywhere in the key, which can block unrelated keys such as authorName or oauthRedirect. Consider adding word/segment boundaries (e.g., start/end, _/- separators) or matching against a list of exact forbidden keys to reduce false positives.
| var sensitiveKeyPattern = /(pass(word)?|secret|token|auth|api[_-]?key|session|cookie|credential|jwt)/i; | |
| var sensitiveKeyPattern = /(pass(word)?|secret|token|(^|[_-])auth([_-]|$)|api[_-]?key|session|cookie|credential|jwt)/i; |
Problem
The storage helper writes raw values/objects directly to
window.localStoragewithout encryption, scope restrictions, or key allowlisting. If credentials, API keys, or session tokens are stored through this helper, they are accessible to any injected script and may persist indefinitely.Severity:
highFile:
www/js/ionicUtils.jsSolution
Do not store secrets in localStorage. Move sensitive values to platform secure storage (Keychain/Keystore via Cordova secure plugins), minimize retention, and enforce key-level policy to prevent accidental secret writes.
Changes
www/js/ionicUtils.js(modified)Testing