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

Prevent xss #2620

Merged
merged 8 commits into from
Jun 28, 2020
9 changes: 7 additions & 2 deletions webapp/graphite/dashboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from graphite.dashboard.models import Dashboard, Template
from graphite.dashboard.send_graph import send_graph_email
from graphite.render.views import renderView
from graphite.util import json
from graphite.util import json, sanitize
from graphite.user_util import isAuthenticated

fieldRegex = re.compile(r'<([^>]+)>')
Expand Down Expand Up @@ -224,9 +224,14 @@ def getPermissions(user):
def save(request, name):
if 'change' not in getPermissions(request.user):
return json_response( dict(error="Must be logged in with appropriate permissions to save") )

# Deserialize and reserialize as a validation step
state = str( json.dumps( json.loads( request.POST['state'] ) ) )
state = json.loads(request.POST['state'])
if state.get("name"):
state["name"] = sanitize(state["name"])
state = str(json.dumps(state))

name = sanitize(name)
try:
dashboard = Dashboard.objects.get(name=name)
except Dashboard.DoesNotExist:
Expand Down
6 changes: 6 additions & 0 deletions webapp/graphite/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

from django.conf import settings
from django.utils.timezone import make_aware
from django.utils.html import strip_tags

from graphite.compat import HttpResponse
from graphite.logger import log
Expand Down Expand Up @@ -394,3 +395,8 @@ def parseHost(host_string):

def parseHosts(host_strings):
return [parseHost(host_string) for host_string in host_strings]


def sanitize(input_string):
"""Sanitize input string to prevent XSS vulnerability attack."""
return strip_tags(input_string)
15 changes: 15 additions & 0 deletions webapp/tests/test_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ def test_dashboard_pass_valid(self):
response = self.client.get(url)
self.assertEqual(response.status_code, 200)

def test_dashboard_for_sanitization_on_save(self):
# Saving dashboard
url = reverse('dashboard_save', args=['testdashboard<img src=x onerror=alert("Boom")>'])
request = {"state": '{}'}
response = self.client.post(url, request)
self.assertEqual(response.status_code, 200)

# Verifying dashboard name was sanitized
url = reverse('dashboard_find')
request = {"query": "test"}
response = self.client.get(url, request)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.content, b'{"dashboards": [{"name": "testdashboard"}]}')

def test_dashboard_pass_invalid_name(self):
url = reverse('dashboard', args=['bogusdashboard'])
response = self.client.get(url)
Expand Down Expand Up @@ -251,6 +265,7 @@ def test_dashboard_save_template(self):
self.assertEqual(response.status_code, 200)

# Save again after it now exists

def test_dashboard_save_template_overwrite(self):
url = reverse('dashboard_save_template', args=['testtemplate', 'testkey'])
request = copy.deepcopy(self.testtemplate)
Expand Down