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

Remove all extra XSS filtering #2712

Merged
merged 2 commits into from
Oct 24, 2018
Merged

Remove all extra XSS filtering #2712

merged 2 commits into from
Oct 24, 2018

Conversation

dafeder
Copy link
Member

@dafeder dafeder commented Oct 24, 2018

After upgrading environments to PHP7, we've noticed that search facet items are being double-escape on characters like ampersands (&). Removing two different XSS filters we had added in fixes this, and we have not been able to reproduce any issues of XSS scripting vulnerability. This can be reversed if security scans detect a problem.

To reproduce

  • Build current release of DKAN on PHP 7.1
  • Create a group with an ampersand in the name
  • View the search page and open the group facet; the ampersand should render as "&" and the link won't work.

QA Steps

  • Build on a server running PHP 7.1
  • Create a group with an ampersand in the name
  • View the search page and open the group facet; the ampersand should render correctly and the link should work.

This also removes any licenses from the licenses facet that aren't defined in DKAN, so that we don't have ugly URLs in the facet (which lead to broken links anyway).

@dafeder dafeder requested a review from janette October 24, 2018 17:17
@dafeder dafeder added this to the Sprint 8 milestone Oct 24, 2018
Copy link
Member

@janette janette left a comment

Choose a reason for hiding this comment

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

Facets look good, and no warnings in the security scan.

@janette janette merged commit 3570835 into 7.x-1.x Oct 24, 2018
@janette janette deleted the rip-out-xss branch October 24, 2018 21:18
dafeder added a commit that referenced this pull request Apr 24, 2020
* Remove all extra XSS filtering

* Hide licenses from facet that don't have keys
dafeder added a commit that referenced this pull request Apr 24, 2020
* Remove all extra XSS filtering

* Hide licenses from facet that don't have keys
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.

2 participants