From 93e751c05825e9ab2f375497cf9b5fc9edf2d186 Mon Sep 17 00:00:00 2001
From: Kyle Mahan <kyle.mahan@gmail.com>
Date: Fri, 6 May 2016 14:33:42 -0700
Subject: [PATCH] escape syndication target before displaying

closes possible XSS vector

fixes #51
---
 woodwind/api.py                  |  5 ++++-
 woodwind/templates/_reply.jinja2 |  2 +-
 woodwind/util.py                 | 11 +++++++++++
 woodwind/views.py                | 25 ++++++++++++++-----------
 4 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/woodwind/api.py b/woodwind/api.py
index 3c1860b..f2eeb78 100644
--- a/woodwind/api.py
+++ b/woodwind/api.py
@@ -1,7 +1,7 @@
 import flask
 import flask.ext.login as flask_login
 import requests
-
+from woodwind import util
 
 api = flask.Blueprint('api', __name__)
 
@@ -13,6 +13,9 @@ def publish():
     content = flask.request.form.get('content')
     syndicate_to = flask.request.form.getlist('syndicate-to[]')
 
+    if syndicate_to:
+        syndicate_to = [util.html_unescape(id) for id in syndicate_to]
+
     data = {
         'h': 'entry',
         'syndicate-to[]': syndicate_to,
diff --git a/woodwind/templates/_reply.jinja2 b/woodwind/templates/_reply.jinja2
index fc30ae4..5b30a96 100644
--- a/woodwind/templates/_reply.jinja2
+++ b/woodwind/templates/_reply.jinja2
@@ -23,7 +23,7 @@
       {% for target in current_user.get_setting('syndicate-to', []) %}
 
         <div class="syndication-toggle">
-          <input id="sc-{{entry.id}}-{{loop.index}}" type="checkbox" name="syndicate-to[]" value="{{ target | syndication_target_id }}"{% if entry is syndicated_to(target) %} checked{% endif %} />
+          <input id="sc-{{entry.id}}-{{loop.index}}" type="checkbox" name="syndicate-to[]" value="{{ target | render_syndication_target_id }}"{% if entry is syndicated_to(target) %} checked{% endif %} />
           <label for="sc-{{entry.id}}-{{loop.index}}">{{ target | render_syndication_target }}</label>
         </div>
 
diff --git a/woodwind/util.py b/woodwind/util.py
index 681d3bf..2acd188 100644
--- a/woodwind/util.py
+++ b/woodwind/util.py
@@ -1,5 +1,6 @@
 import pickle
 import re
+from xml.sax import saxutils
 
 from flask import current_app
 from redis import StrictRedis
@@ -56,3 +57,13 @@ def clean(text):
     if text is not None:
         text = re.sub('<script.*?</script>', '', text, flags=re.DOTALL)
         return bleach.clean(text, strip=True)
+
+
+def html_escape(text):
+    # https://wiki.python.org/moin/EscapingHtml
+    return saxutils.escape(text, {'"': '&quot;', "'": '&apos;'})
+
+
+def html_unescape(text):
+    # https://wiki.python.org/moin/EscapingHtml
+    return saxutils.unescape(text, {'&quot;': '"', '&apos;': "'"})
diff --git a/woodwind/views.py b/woodwind/views.py
index 2ec325c..88e7065 100644
--- a/woodwind/views.py
+++ b/woodwind/views.py
@@ -16,7 +16,6 @@ import pyquerystring
 import requests
 import re
 import urllib
-import cgi
 import sqlalchemy
 import sqlalchemy.sql.expression
 
@@ -257,11 +256,11 @@ def login():
 @micropub.authenticated_handler
 def login_callback(resp):
     if not resp.me:
-        flask.flash(cgi.escape('Login error: ' + resp.error))
+        flask.flash(util.html_escape('Login error: ' + resp.error))
         return flask.redirect(flask.url_for('.index'))
 
     if resp.error:
-        flask.flash(cgi.escape('Warning: ' + resp.error))
+        flask.flash(util.html_escape('Warning: ' + resp.error))
 
     user = load_user(resp.me)
     if not user:
@@ -287,12 +286,12 @@ def authorize():
 @micropub.authorized_handler
 def micropub_callback(resp):
     if not resp.me or resp.error:
-        flask.flash(cgi.escape('Authorize error: ' + resp.error))
+        flask.flash(util.html_escape('Authorize error: ' + resp.error))
         return flask.redirect(flask.url_for('.index'))
 
     user = load_user(resp.me)
     if not user:
-        flask.flash(cgi.escape('Unknown user for url: ' + resp.me))
+        flask.flash(util.html_escape('Unknown user for url: ' + resp.me))
         return flask.redirect(flask.url_for('.index'))
 
     user.micropub_endpoint = resp.micropub_endpoint
@@ -764,21 +763,25 @@ def font_awesome_class_for_service(service):
     return 'fa fa-send'
 
 
-@views.app_template_filter('syndication_target_id')
-def syndication_target_id(target):
+@views.app_template_filter('render_syndication_target_id')
+def render_syndication_target_id(target):
     if isinstance(target, dict):
-        return target.get('uid') or target.get('id')
-    return target
+        id = target.get('uid') or target.get('id')
+    else:
+        id = target
+    return util.html_escape(id)
 
 
 @views.app_template_filter('render_syndication_target')
 def render_syndication_target(target):
     if isinstance(target, dict):
         full_name = target.get('name')
-        return full_name
+        return util.html_escape(full_name)
 
     return '<img src="{}" alt="{}" />&nbsp;{}'.format(
-        favicon_for_url(target), target, prettify_url(target))
+        favicon_for_url(target),
+        util.html_escape(target),
+        util.html_escape(prettify_url(target)))
 
 
 @views.app_template_test('syndicated_to')