From 25698a7b6cc547a7a33dba312081a129195ee629 Mon Sep 17 00:00:00 2001
From: Kevin Chung <kchung@nyu.edu>
Date: Wed, 20 Dec 2017 00:11:42 -0500
Subject: [PATCH] Sandbox themes (#534)

* Use the Jinja sandbox to render themes
* Prevent get_config from accessing config.py values
* Add get_app_config
* Add tests for sandbox
---
 CTFd/__init__.py     | 10 ++++++++--
 CTFd/utils.py        | 25 ++++++++++---------------
 tests/test_themes.py | 43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 17 deletions(-)
 create mode 100644 tests/test_themes.py

diff --git a/CTFd/__init__.py b/CTFd/__init__.py
index 65f31dbe..43665607 100644
--- a/CTFd/__init__.py
+++ b/CTFd/__init__.py
@@ -3,7 +3,8 @@ import os
 
 from distutils.version import StrictVersion
 from flask import Flask
-from jinja2 import FileSystemLoader
+from jinja2 import FileSystemLoader, select_autoescape
+from jinja2.sandbox import SandboxedEnvironment
 from sqlalchemy.engine.url import make_url
 from sqlalchemy.exc import OperationalError, ProgrammingError
 from sqlalchemy_utils import database_exists, create_database
@@ -63,7 +64,12 @@ def create_app(config='CTFd.config.Config'):
     app = Flask(__name__)
     with app.app_context():
         app.config.from_object(config)
-        app.jinja_loader = ThemeLoader(os.path.join(app.root_path, 'themes'), followlinks=True)
+        theme_loader = ThemeLoader(os.path.join(app.root_path, 'themes'), followlinks=True)
+        app.jinja_env = SandboxedEnvironment(
+            loader=theme_loader,
+            autoescape=select_autoescape(['html', 'xml'])
+        )
+        app.jinja_loader = theme_loader
 
         from CTFd.models import db, Teams, Solves, Challenges, WrongKeys, Keys, Tags, Files, Tracking
 
diff --git a/CTFd/utils.py b/CTFd/utils.py
index ca015c27..d08a5297 100644
--- a/CTFd/utils.py
+++ b/CTFd/utils.py
@@ -516,18 +516,13 @@ def delete_file(file_id):
 
 
 @cache.memoize()
-def get_config(key):
+def get_app_config(key):
     value = app.config.get(key)
-    if value:
-        if value and value.isdigit():
-            return int(value)
-        elif value and isinstance(value, six.string_types):
-            if value.lower() == 'true':
-                return True
-            elif value.lower() == 'false':
-                return False
-            else:
-                return value
+    return value
+
+
+@cache.memoize()
+def get_config(key):
     config = Config.query.filter_by(key=key).first()
     if config and config.value:
         value = config.value
@@ -774,7 +769,7 @@ def update_check(force=False):
 
 
 def export_ctf(segments=None):
-    db = dataset.connect(get_config('SQLALCHEMY_DATABASE_URI'))
+    db = dataset.connect(get_app_config('SQLALCHEMY_DATABASE_URI'))
     if segments is None:
         segments = ['challenges', 'teams', 'both', 'metadata']
 
@@ -826,7 +821,7 @@ def export_ctf(segments=None):
         backup_zip.writestr('db/alembic_version.json', result_file.read())
 
     # Backup uploads
-    upload_folder = os.path.join(os.path.normpath(app.root_path), get_config('UPLOAD_FOLDER'))
+    upload_folder = os.path.join(os.path.normpath(app.root_path), app.config.get('UPLOAD_FOLDER'))
     for root, dirs, files in os.walk(upload_folder):
         for file in files:
             parent_dir = os.path.basename(root)
@@ -838,7 +833,7 @@ def export_ctf(segments=None):
 
 
 def import_ctf(backup, segments=None, erase=False):
-    side_db = dataset.connect(get_config('SQLALCHEMY_DATABASE_URI'))
+    side_db = dataset.connect(get_app_config('SQLALCHEMY_DATABASE_URI'))
     if segments is None:
         segments = ['challenges', 'teams', 'both', 'metadata']
 
@@ -924,7 +919,7 @@ def import_ctf(backup, segments=None, erase=False):
                     entry_id = entry.pop('id', None)
                     # This is a hack to get SQlite to properly accept datetime values from dataset
                     # See Issue #246
-                    if get_config('SQLALCHEMY_DATABASE_URI').startswith('sqlite'):
+                    if get_app_config('SQLALCHEMY_DATABASE_URI').startswith('sqlite'):
                         for k, v in entry.items():
                             if isinstance(v, six.string_types):
                                 match = re.match(r"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d", v)
diff --git a/tests/test_themes.py b/tests/test_themes.py
new file mode 100644
index 00000000..e4133e3e
--- /dev/null
+++ b/tests/test_themes.py
@@ -0,0 +1,43 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+from tests.helpers import *
+from jinja2.sandbox import SecurityError
+
+
+def test_themes_run_in_sandbox():
+    """Does get_config and set_config work properly"""
+    app = create_ctfd()
+    with app.app_context():
+        try:
+            app.jinja_env.from_string("{{ ().__class__.__bases__[0].__subclasses__()[40]('./test_utils.py').read() }}").render()
+        except SecurityError:
+            pass
+        except Exception as e:
+            raise e
+    destroy_ctfd(app)
+
+
+def test_themes_cant_access_configpy_attributes():
+    """Themes should not be able to access config.py attributes"""
+    app = create_ctfd()
+    with app.app_context():
+        assert app.config['SECRET_KEY'] == 'AAAAAAAAAAAAAAAAAAAA'
+        assert app.jinja_env.from_string("{{ get_config('SECRET_KEY') }}").render() != app.config['SECRET_KEY']
+    destroy_ctfd(app)
+
+
+def test_themes_escape_html():
+    """Themes should escape XSS properly"""
+    app = create_ctfd()
+    with app.app_context():
+        team = gen_team(app.db, name="<script>alert(1)</script>")
+        team.affiliation = "<script>alert(1)</script>"
+        team.website = "<script>alert(1)</script>"
+        team.country = "<script>alert(1)</script>"
+
+        with app.test_client() as client:
+            r = client.get('/teams')
+            assert r.status_code == 200
+            assert "<script>alert(1)</script>" not in r.get_data(as_text=True)
+    destroy_ctfd(app)