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

Add plugin to append a simple serialize and deserialize to stores #104

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fkrauthan
Copy link

This is a pull request to solve ticket #102
This plugin adds a simple json serialize and deserialize to a Store.

@fkrauthan
Copy link
Author

Any timeline when you gonna take a look at my pull request?

@acdlite
Copy link
Owner

acdlite commented Apr 5, 2015

This implementation is susceptible to XSS attacks if any of the stores contain HTML strings.

@fkrauthan
Copy link
Author

How exactly? This plugin stores stuff exactly as they are in the current state and restores exact that state. Don't think serialization and de-serialization needs to handle XSS

@jbmusso
Copy link

jbmusso commented Apr 13, 2015

Interesting topic. Would using a library such as https://github.com/leizongmin/js-xss or https://github.com/cure53/DOMPurify help with the XSS vulnerability?

-edit- I forgot to mention https://github.com/yahoo/xss-filters as well

@fkrauthan
Copy link
Author

Once again I think that would be a huge mistake for this use case. You don't want any data manipulation for serialization and deserialization. The main purpose is that what I put in comes out again and the other way around. Some sort of purify could be done as a separate plugin.

@jbmusso
Copy link

jbmusso commented Apr 13, 2015

@fkrauthan I have limited knowledge on the topic. Are you referring to the fact that data should be sanitized prior to being inserted in the database (typically)?

@fkrauthan
Copy link
Author

@jbmusso I am referring to the fact that sanitization should not happen on a save and restore functionality. This pull request is a wrapper for registering serialize and deserialize function automatically for your models based on a toJSON and fromJSON functionality. The main goal of that is to save the current state (exactly as it is) and restores it (exactly as it is). Any attempt to sanitize data would possible change the data and destroy the purpose of serialize and deserialize.

@jbmusso
Copy link

jbmusso commented Apr 13, 2015

@fkrauthan Thanks, this makes sense. I also feel that this implies that one should make sure that no state could contain data that, once serialized, would be possible vectors for a XSS attack. This leaves the responsibility to the developer to sanitize all strings prior to serializing the state, and that'd make your plugin useful. Let me know if I'm mistaken.

@fkrauthan
Copy link
Author

@jbmusso No that is exactly what I want. If people are concert about unsafe data they could catch it and sanitize before writing it to the store (setState on the store). And since it is a optional plugin no one gets force the shortcut to get a simple serialize and deserialize working. It is really just a short cut that will work for 99% of the users who need state dehydration on server site and rehydration on client site.

@apaatsio
Copy link

+1 for this PR. I would rename the restorableStateStorePlugin to serializableStorePlugin, though.

I think this is a wrong place to handle XSS. The user should be able assume that x equals deserialize(serialize(x)).

@cr0cK
Copy link

cr0cK commented May 1, 2015

+1 as well.

@voronianski
Copy link
Collaborator

Is it still actual for someone?

@fkrauthan
Copy link
Author

I still would like to have this in flummox

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

Successfully merging this pull request may close these issues.

6 participants