diff --git a/poppy/storage/mockdb/services.py b/poppy/storage/mockdb/services.py index b2fc52eb..c7cd088e 100644 --- a/poppy/storage/mockdb/services.py +++ b/poppy/storage/mockdb/services.py @@ -53,7 +53,7 @@ class ServicesController(base.ServicesController): services = [] for i in self.created_service_ids: services = [{'service_id': i, - 'name': uuid.uuid4(), + 'service_name': uuid.uuid4(), 'domains': [json.dumps( {'domain': 'www.mywebsite.com'}) ], @@ -114,7 +114,7 @@ class ServicesController(base.ServicesController): [{'operator_url': 'mockcf123.fastly.prod.com'}]})} service_dict = {'service_id': service_id, - 'name': uuid.uuid4(), + 'service_name': uuid.uuid4(), 'domains': [domain_json], 'origins': [origin_json], 'flavor_id': 'standard', @@ -185,7 +185,7 @@ class ServicesController(base.ServicesController): @staticmethod def format_result(result): service_id = result.get('service_id') - name = result.get('service_name') + name = str(result.get('service_name')) origins = [json.loads(o) for o in result.get('origins', [])] domains = [json.loads(d) for d in result.get('domains', [])] origins = [origin.Origin(o['origin'], diff --git a/poppy/transport/pecan/controllers/v1/flavors.py b/poppy/transport/pecan/controllers/v1/flavors.py index cf1686cd..3712f538 100644 --- a/poppy/transport/pecan/controllers/v1/flavors.py +++ b/poppy/transport/pecan/controllers/v1/flavors.py @@ -51,6 +51,11 @@ class FlavorsController(base.Controller, hooks.HookController): } @pecan.expose('json') + @decorators.validate( + flavor_id=rule.Rule( + helpers.is_valid_flavor_id(), + helpers.abort_with_message) + ) def get_one(self, flavor_id): """get_one diff --git a/poppy/transport/pecan/models/response/cachingrules.py b/poppy/transport/pecan/models/response/cachingrules.py index 52873ef8..f1fffefe 100644 --- a/poppy/transport/pecan/models/response/cachingrules.py +++ b/poppy/transport/pecan/models/response/cachingrules.py @@ -12,7 +12,7 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. - +import cgi try: import ordereddict as collections except ImportError: # pragma: no cover @@ -27,7 +27,7 @@ class Model(collections.OrderedDict): def __init__(self, caching): super(Model, self).__init__() - self['name'] = caching.name + self['name'] = cgi.escape(caching.name) self['ttl'] = caching.ttl if caching.rules != []: self['rules'] = [rule.Model(r) for r in caching.rules] diff --git a/poppy/transport/pecan/models/response/domain.py b/poppy/transport/pecan/models/response/domain.py index e9cdc7fd..6eb48a6c 100644 --- a/poppy/transport/pecan/models/response/domain.py +++ b/poppy/transport/pecan/models/response/domain.py @@ -12,7 +12,7 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. - +import cgi try: import ordereddict as collections except ImportError: # pragma: no cover @@ -25,5 +25,5 @@ class Model(collections.OrderedDict): def __init__(self, domain): super(Model, self).__init__() - self['domain'] = domain.domain + self['domain'] = cgi.escape(domain.domain) self['protocol'] = domain.protocol diff --git a/poppy/transport/pecan/models/response/origin.py b/poppy/transport/pecan/models/response/origin.py index 6f576013..09c4f8aa 100644 --- a/poppy/transport/pecan/models/response/origin.py +++ b/poppy/transport/pecan/models/response/origin.py @@ -12,7 +12,7 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. - +import cgi try: import ordereddict as collections except ImportError: # pragma: no cover @@ -27,7 +27,7 @@ class Model(collections.OrderedDict): def __init__(self, origin): super(Model, self).__init__() - self['origin'] = origin.origin + self['origin'] = cgi.escape(origin.origin) self['port'] = origin.port self['ssl'] = origin.ssl self['rules'] = [rule.Model(r) for r in origin.rules] diff --git a/poppy/transport/pecan/models/response/restriction.py b/poppy/transport/pecan/models/response/restriction.py index 4744d7fe..742c0c0e 100644 --- a/poppy/transport/pecan/models/response/restriction.py +++ b/poppy/transport/pecan/models/response/restriction.py @@ -12,7 +12,7 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. - +import cgi try: import ordereddict as collections except ImportError: # pragma: no cover @@ -27,5 +27,5 @@ class Model(collections.OrderedDict): def __init__(self, restriction): super(Model, self).__init__() - self['name'] = restriction.name + self['name'] = cgi.escape(restriction.name) self['rules'] = [rule.Model(r) for r in restriction.rules] diff --git a/poppy/transport/pecan/models/response/rule.py b/poppy/transport/pecan/models/response/rule.py index f025954e..aaaeccc2 100644 --- a/poppy/transport/pecan/models/response/rule.py +++ b/poppy/transport/pecan/models/response/rule.py @@ -12,7 +12,7 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. - +import cgi try: import ordereddict as collections except ImportError: # pragma: no cover @@ -25,10 +25,10 @@ class Model(collections.OrderedDict): def __init__(self, rule): super(Model, self).__init__() - self['name'] = rule.name + self['name'] = cgi.escape(rule.name) for attr_name in ['http_host', 'http_method', 'client_ip', 'request_url', 'referrer']: attr = getattr(rule, attr_name, None) if attr is not None: - self[attr_name] = attr + self[attr_name] = cgi.escape(attr) diff --git a/poppy/transport/pecan/models/response/service.py b/poppy/transport/pecan/models/response/service.py index c21a89c0..08d4a4b8 100644 --- a/poppy/transport/pecan/models/response/service.py +++ b/poppy/transport/pecan/models/response/service.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import cgi try: import ordereddict as collections except ImportError: # pragma: no cover @@ -32,7 +33,7 @@ class Model(collections.OrderedDict): def __init__(self, service_obj, controller): super(Model, self).__init__() - self["name"] = service_obj.name + self["name"] = cgi.escape(service_obj.name) self["id"] = str(service_obj.service_id) self["domains"] = [domain.Model(d) for d in service_obj.domains] self["origins"] = [origin.Model(o) for o in service_obj.origins] diff --git a/tests/api/services/data_create_service_xss.json b/tests/api/services/data_create_service_xss.json new file mode 100644 index 00000000..7984a8c9 --- /dev/null +++ b/tests/api/services/data_create_service_xss.json @@ -0,0 +1,75 @@ +{ + "name_injection": { + "name": "", + "domain_list": [{"domain": "mywebsite.com", "protocol": "http"}], + "origin_list": [{"origin": "mywebsite1.com", + "port": 443, + "ssl": false}], + "caching_list": [{"name": "default", "ttl": 3600}, + {"name": "home", + "ttl": 1200, + "rules": [{"name" : "index", + "request_url" : "/index.htm"}]}] + }, + "domain_injection": { + "name": "bad_domain", + "domain_list": [ + { + "domain": "", + "protocol": "http" + } + ], + "origin_list": [{"origin": "mywebsite1.com", + "port": 443, + "ssl": false}], + "caching_list": [] + }, + "origin_injection": { + "name": "bad_origin", + "domain_list": [{"domain": "mywebsite.com", "protocol": "http"}], + "origin_list": [{"origin": "", + "port": 443, + "ssl": false}], + "caching_list": [{"name": "default", "ttl": 3600}, + {"name": "home", + "ttl": 1200, + "rules": [{"name" : "index", + "request_url" : "/index.htm"}]}] + }, + "caching_name_injection": { + "name": "bad_caching_name", + "domain_list": [{"domain": "mywebsite.com", "protocol": "http"}], + "origin_list": [{"origin": "mywebsite1.com", + "port": 443, + "ssl": false}], + "caching_list": [{"name": "default", "ttl": 3600}, + {"name": "", + "ttl": 1200, + "rules": [{"name" : "index", + "request_url" : "/index.htm"}]}] + }, + "caching_rule_injection": { + "name": "bad_caching_name", + "domain_list": [{"domain": "mywebsite.com", "protocol": "http"}], + "origin_list": [{"origin": "mywebsite1.com", + "port": 443, + "ssl": false}], + "caching_list": [{"name": "default", "ttl": 3600}, + {"name": "images", + "ttl": 1200, + "rules": [{"name" : "index", + "request_url" : ""}]}] + }, + "caching_rule_name_injection": { + "name": "bad_caching_name", + "domain_list": [{"domain": "mywebsite.com", "protocol": "http"}], + "origin_list": [{"origin": "mywebsite1.com", + "port": 443, + "ssl": false}], + "caching_list": [{"name": "default", "ttl": 3600}, + {"name": "images", + "ttl": 1200, + "rules": [{"name" : "", + "request_url" : "/images"}]}] + } +} diff --git a/tests/api/services/test_services.py b/tests/api/services/test_services.py index 7be7edc9..32c5064c 100644 --- a/tests/api/services/test_services.py +++ b/tests/api/services/test_services.py @@ -15,6 +15,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import cgi import time import uuid @@ -142,6 +143,78 @@ class TestCreateService(providers.TestProviderBase): super(TestCreateService, self).tearDown() + @ddt.file_data("data_create_service_xss.json") + def test_create_service_with_xss_injection(self, test_data): + # create with hacker data + service_name = test_data['name'] + domain_list = test_data['domain_list'] + for item in domain_list: + item['domain'] = str(uuid.uuid1()) + '.com' + + origin_list = test_data['origin_list'] + caching_list = test_data['caching_list'] + if 'flavor_id' in test_data: + flavor_id = test_data['flavor_id'] + else: + flavor_id = self.flavor_id + + resp = self.client.create_service(service_name=service_name, + domain_list=domain_list, + origin_list=origin_list, + caching_list=caching_list, + flavor_id=flavor_id) + if 'location' in resp.headers: + self.service_url = resp.headers['location'] + + self.assertEqual(resp.status_code, 202) + self.assertEqual(resp.text, '') + self.service_url = resp.headers['location'] + + resp = self.client.get_service(location=self.service_url) + self.assertEqual(resp.status_code, 200) + + body = resp.json() + + # validate name + self.assertEqual( + body['name'], + cgi.escape(test_data['name']) + ) + + # validate domain + self.assertEqual( + body['domains'][0]['domain'], + cgi.escape(domain_list[0]['domain']) + ) + + # validate origin + self.assertEqual( + body['origins'][0]['origin'], + cgi.escape(origin_list[0]['origin']) + ) + + if len(caching_list) > 0: + # validate caching name + self.assertEqual( + body['caching'][1]['name'], + cgi.escape(caching_list[1]['name']) + ) + + if len(caching_list) > 1: + # we have more than just the default + if len(caching_list[1]['rules']) > 0: + # validate caching rule name + self.assertEqual( + body['caching'][1]['rules'][0]['name'], + cgi.escape(caching_list[1]['rules'][0]['name']) + ) + + # validate caching rule request_url + self.assertEqual( + body['caching'][1]['rules'][0]['request_url'], + cgi.escape(caching_list[1]['rules'][0]['request_url']) + ) + @ddt.ddt class TestListServices(base.TestBase):