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

Upgrade to the latest OWASP HTML sanitizer #50765

Merged
merged 3 commits into from
Jan 17, 2020

Conversation

jaymode
Copy link
Member

@jaymode jaymode commented Jan 8, 2020

This commit upgrades the OWASP HTML sanitizer used by watcher to the
latest version and also upgrades guava, which it depends on. The guava
upgrade also requires the addition of a new dependency that guava
itself requires as of version 27.0.

Relates #50395

This commit upgrades the OWASP HTML sanitizer used by watcher to the
latest version and also upgrades guava, which it depends on. The guava
upgrade also requires the addition of a new dependency that guava
itself requires as of version 27.0.

Relates elastic#50395
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Watcher)

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@jaymode
Copy link
Member Author

jaymode commented Jan 9, 2020

I just pushed a change that removes a test that checks the behavior when the html sanitizer is used with templated values like {{ctx.metadata.name}}. The sanitizer's behavior has changed to re-write these templated values with a comment, {<!-- -->{ctx.metadata.name}}. This would be an issue if we attempted to sanitize the template, but the code that uses the sanitizer runs the rendered string through the sanitizer, which means that the templated values have been replaced already. The behavior being tested by the test wasn't technically possible or usable based on my understanding.

@jaymode jaymode requested a review from rjernst January 9, 2020 17:48
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

Still LGTM

@polyfractal polyfractal added v7.7.0 and removed v7.6.0 labels Jan 15, 2020
@jaymode
Copy link
Member Author

jaymode commented Jan 16, 2020

@elasticmachine update branch

@jaymode jaymode merged commit 1bec37d into elastic:master Jan 17, 2020
@jaymode jaymode deleted the watcher_guava_upgrade branch January 17, 2020 15:57
jaymode added a commit to jaymode/elasticsearch that referenced this pull request Jan 17, 2020
This commit upgrades the OWASP HTML sanitizer used by watcher to the
latest version and also upgrades guava, which it depends on. The guava
upgrade also requires the addition of a new dependency that guava
itself requires as of version 27.0. The sanitizer's behavior has changed to
re-write these templated values with a comment that results in this output
`{<!-- -->{ctx.metadata.name}}`. This would be an issue if we attempted to
sanitize the template, but the code that uses the sanitizer runs the rendered
string through the sanitizer, which means that the templated values have
been replaced already.

Relates elastic#50395
jaymode added a commit that referenced this pull request Jan 17, 2020
This commit upgrades the OWASP HTML sanitizer used by watcher to the
latest version and also upgrades guava, which it depends on. The guava
upgrade also requires the addition of a new dependency that guava
itself requires as of version 27.0. The sanitizer's behavior has changed to
re-write these templated values with a comment that results in this output
`{<!-- -->{ctx.metadata.name}}`. This would be an issue if we attempted to
sanitize the template, but the code that uses the sanitizer runs the rendered
string through the sanitizer, which means that the templated values have
been replaced already.

Relates #50395
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
This commit upgrades the OWASP HTML sanitizer used by watcher to the
latest version and also upgrades guava, which it depends on. The guava
upgrade also requires the addition of a new dependency that guava
itself requires as of version 27.0. The sanitizer's behavior has changed to
re-write these templated values with a comment that results in this output
`{<!-- -->{ctx.metadata.name}}`. This would be an issue if we attempted to
sanitize the template, but the code that uses the sanitizer runs the rendered
string through the sanitizer, which means that the templated values have
been replaced already.

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

Successfully merging this pull request may close these issues.

None yet

5 participants