Skip to content
This repository has been archived by the owner on Mar 23, 2022. It is now read-only.

Fix XSS on term-detailed web client view #51

Merged

Conversation

billbogaiv
Copy link
Member

Previous behavior would render unencoded HTML in the view based on the term's definition-data.
New behavior encodes HTML-tags in the DB-module via a separate sanitize module.
However, be aware this functionality should be called prior to any markdown-related conversion in the future.
Otherwise, the resulting HTML will be encoded.
Related to #49.

Also added a new gulp task for running TDD-style tests.
And, incorporated this into the existing test-task.

Previous behavior would render unencoded HTML in the view based on the term's definition-data.
New behavior encodes HTML-tags in the DB-module via a separate sanitize module.
However, be aware this functionality should be called prior to any markdown-related conversion in the future.
Otherwise, the resulting HTML will be encoded.
Related to ritterim#49.

Also added a new gulp task for running TDD-style tests.
And, incorporated this into the existing test-task.
@billbogaiv billbogaiv force-pushed the fix-xss-in-terms-detailed-web-client branch from 45f497f to b31a022 Compare January 24, 2015 06:08
@billbogaiv billbogaiv changed the title [WIP] Fix XSS on term-detailed web client view Fix XSS on term-detailed web client view Jan 24, 2015
gulp.task('default', ['serve-dev'])

gulp.task('test', ['build'], function () {
Copy link
Member Author

Choose a reason for hiding this comment

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

Don't know why there were two test tasks. This one will never execute. @allenbrubaker, any ideas?

kendaleiv added a commit that referenced this pull request Jan 26, 2015
…eb-client

Fix XSS on term-detailed web client view
@kendaleiv kendaleiv merged commit e8a8d9a into ritterim:master Jan 26, 2015
@billbogaiv billbogaiv deleted the fix-xss-in-terms-detailed-web-client branch January 26, 2015 14:10
@billbogaiv
Copy link
Member Author

Due to the introduction of #54, doing the conversion in the DB-module doesn't feel like the best place for sanitizing HTML-output. This has an impact on API-calls too, which may want to preserve the original data-format. Ultimately, it should be up to the client to determine what to do what the data. Therefore, the aforementioned PR will contain refactoring of this logic.

@billbogaiv billbogaiv mentioned this pull request Jan 26, 2015
billbogaiv added a commit to billbogaiv/definely that referenced this pull request Jan 26, 2015
Add new set of handlebars helpers and refactor existing helper into new module.
Update terms/show view to sanitize HTML and convert result into HTML via marked NPM.
Also refactor database module related to ritterim#51.
New behavior expects any sanitizing to be client-specific.
Related to ritterim#54 and ritterim#53.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants