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

fix: sanitise sharedb node data using dompurify #2478

Closed
wants to merge 4 commits into from

Conversation

jessicamcinchak
Copy link
Member

@jessicamcinchak jessicamcinchak commented Nov 23, 2023

Testing notes in devops email

Changes:

  • When creating an _add or _update operation against SharedDB, we would already "sanitize" values using custom regex to handle empty values. I've added an extra step here to additionally apply DOMPurify.sanitize() on the user data inputs as well

Todo:

Copy link

github-actions bot commented Nov 23, 2023

Removed vultr server and associated DNS entries

return trim(x.replace(/[\u200B-\u200D\uFEFF↵]/g, ""));
} else if ((x && typeof x === "object") || x instanceof Object) {
return Object.entries(x).reduce((acc, [k, v]) => {
v = sanitize(v);
acc[k] = v;
Copy link
Member Author

@jessicamcinchak jessicamcinchak Nov 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block only return acc, and v = sanitize(v) was indeed sanitizing, but not actually updating the acc which is returned:

  • In cases of handling empty values, this didn't matter because we delete acc[k]
  • But in cases of sanitising values we want to keep, we need to update the return value too

@jessicamcinchak
Copy link
Member Author

Replaced by #2483

@jessicamcinchak jessicamcinchak deleted the jess/dom-purify-sharedb-ops branch November 27, 2023 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant