gsklee / ngStorage

localStorage and sessionStorage done right for AngularJS.
MIT License
2.33k stars 461 forks source link

Prevent XSS Tampering #193

Open SamuelMS opened 8 years ago

SamuelMS commented 8 years ago

ngStorage crashes my entire application when I replace its storage entry with an empty/undefined value.

What's to prevent attackers from replacing the ngStorage-sidebar (prefixed or otherwise) storage entry with external tools? I can ensure data that I explicitly load from $localStorage and $sessionStorage is valid on my end, but ngStorage itself seems to explode when its stored value isn't of the expected format.

In other words, ngStorage throws an exception when it finds its key in storage, but that key has no value attached to it.

egilkh commented 8 years ago

Should safeguard parsing of key to stop the exploding from being a crash. But that would not stop others from creating the explosion in the first place.

As we have no control over what the browser/device manipulates on it's own we can't stop tampering.

egilkh commented 8 years ago

What's to prevent attackers from replacing the ngStorage-sidebar (prefixed or otherwise) storage entry with external tools?

There is no such prevention. Unless you create your own serializer / deserializer using some sort of crypt/decrypt. Which would need for the key/secret to come from somewhere safe.

SamuelMS commented 8 years ago

@egilkh Sure, but at the very least I would expect that ngStorage could wrap its loading methods in try/catch blocks so that it doesn't explode when it doesn't find an expected, sane value -- because as of the original post, it would prevent my entire application from loading (instead of throwing a warning and moving on).

egilkh commented 8 years ago

Agreed. I'll put safeguards (try/catch) to the serialize / deserialize parts.

I didn't mean to close the issue. Re-opening.