Merge pull request #547 from CityOfNewYork/xss-add-note-fix

Fix XSS on Add Note

app/models.py

@@ -344,15 +344,15 @@ class Users(UserMixin, db.Model):
     active = db.Column(db.Boolean, default=False)
     is_anonymous_requester = db.Column(db.Boolean)
     is_super = db.Column(db.Boolean, nullable=False, default=False)
-    first_name = db.Column(db.String(32), nullable=False)
+    first_name = db.Column(db.String(128), nullable=False)
     middle_initial = db.Column(db.String(1))
-    last_name = db.Column(db.String(64), nullable=False)
+    last_name = db.Column(db.String(128), nullable=False)
     email = db.Column(db.String(254))
     notification_email = db.Column(db.String(254), nullable=True, default=None)
     email_validated = db.Column(db.Boolean(), nullable=False)
     terms_of_use_accepted = db.Column(db.Boolean)
-    title = db.Column(db.String(64))
-    organization = db.Column(db.String(128))  # Outside organization
+    title = db.Column(db.String(256))
+    organization = db.Column(db.String(256))  # Outside organization
     phone_number = db.Column(db.String(25))
     fax_number = db.Column(db.String(25))
     _mailing_address = db.Column(
@@ -761,8 +761,8 @@ class Requests(db.Model):
     category = db.Column(
         db.String, default="All", nullable=False
     )  # FIXME: should be nullable, 'All' shouldn't be used
-    title = db.Column(db.String(90))
-    description = db.Column(db.String(5000))
+    title = db.Column(db.String(256))
+    description = db.Column(db.String(10240))
     date_created = db.Column(db.DateTime, default=datetime.utcnow())
     date_submitted = db.Column(
         db.DateTime

app/request/api/views.py

@@ -268,6 +268,7 @@ def get_request_responses():
                 template_path + 'row.html',
                 response=response,
                 row_num=start + row_count,
+                row_html_id='response-row-{}'.format(str(row_count)),
                 response_type=response_type,
                 determination_type=determination_type,
                 show_preview=not (response.type == response_type.DETERMINATION and
@@ -283,6 +284,7 @@ def get_request_responses():
                         template_path, response.type
                     ),
                     response=response,
+                    modal_html_id="response-modal-body-{}".format(str(row_count)),
                     privacies=[response_privacy.RELEASE_AND_PUBLIC,
                                response_privacy.RELEASE_AND_PRIVATE,
                                response_privacy.PRIVATE],

app/request/views.py

@@ -132,7 +132,7 @@ def new():
                 category=None,
                 agency_ein=(
                     escape(form.request_agency.data)
-                    if form.request_agency.data is not None
+                    if form.request_agency.data != "None"
                     else current_user.default_agency_ein
                 ),
                 submission=escape(form.method_received.data),

app/response/utils.py

@@ -8,6 +8,7 @@
 import json
 from datetime import datetime
 from urllib.parse import urljoin, urlencode
+from lxml.html.clean import clean_html
 
 import os
 import re
@@ -20,7 +21,8 @@
     render_template_string,
     url_for,
     jsonify,
-    Markup
+    Markup,
+    escape
 )
 from flask_login import current_user
 from sqlalchemy.orm import joinedload
@@ -147,6 +149,10 @@ def add_note(request_id, note_content, email_content, privacy, is_editable, is_r
     :param is_requester: requester is creator of the note
 
     """
+    raw_string = Markup(note_content).unescape()
+    cleaned_string = clean_html(raw_string)
+    note_content = escape(cleaned_string)
+
     response = Notes(request_id, privacy, note_content, is_editable=is_editable)
     create_object(response)
     create_response_event(event_type.NOTE_ADDED, response)

app/search/utils.py

@@ -40,6 +40,13 @@ def delete_index():
     """
     es.indices.delete(current_app.config["ELASTICSEARCH_INDEX"], ignore=[400, 404])
 
+def delete_doc(request_id):
+    """
+    Delete a specific doc in the index.
+    """
+    es.delete(index=current_app.config['ELASTICSEARCH_INDEX'],
+              doc_type="request",
+              id=request_id)
 
 def delete_docs():
     """

app/templates/request/responses/modal_body/notes.html

@@ -28,9 +28,7 @@
                 5000 characters remaining
             </p>
         {% else %}
-            <div class="request-description-text">
-                {{ response.content | safe }}
-            </div>
+            <div class="request-description-text" id="{{ modal_html_id }}"></div>
         {% endif %}
 
         {% if edit_response_privacy_permission %}
@@ -48,7 +46,13 @@
         {% endif %}
     </form>
 {% else %}
-    <div class="request-description-text">
-        {{ response.content | safe }}
-    </div>
-{% endif %}
+    <div class="request-description-text" id="{{ modal_html_id }}"></div>
+{% endif %}
+
+<script>
+    {% if not ((edit_response_permission or edit_privacy_response_permission) and is_editable and current_request.status != request_status.CLOSED) or not edit_response_permission %}
+        let responseModalBody = $('#' + '{{ modal_html_id }}');
+        responseModalBody.html({{ response.content | tojson }});
+        responseModalBody.html(responseModalBody.text());
+    {% endif %}
+</script>

app/templates/request/responses/row.html

@@ -12,20 +12,20 @@
             {{ moment(response.event_timestamp).format('dddd, MM/DD/YYYY [at] h:mm A') }}
         </div>
         <div class="row">
-            <div class="col-md-10 metadata-preview">
-                {% if show_preview and response.preview is not none %}
-                    {{ response.preview | safe }}
-                {% endif %}
-            </div>
+            <div class="col-md-10 metadata-preview" id="{{ row_html_id }}"></div>
         </div>
     {% else %}
-        <div class="col-md-6 metadata-preview">
-            {% if show_preview and response.preview is not none %}
-                {{ response.preview | safe }}
-            {% endif %}
-        </div>
+        <div class="col-md-6 metadata-preview" id="{{ row_html_id }}"></div>
         <div class="col-md-3">
             {{ moment(response.date_modified).format('dddd, MM/DD/YYYY [at] h:mm A') }}
         </div>
     {% endif %}
-</div>
+</div>
+
+<script>
+    {% if show_preview and response.preview is not none %}
+        let responseRow = $('#'+ '{{ row_html_id }}');
+        responseRow.html({{ response.preview | tojson }});
+        responseRow.html(responseRow.text());
+    {% endif %}
+</script>

migrations/versions/a7635f23e449_updated_character_limits_for_escaped_.py

@@ -0,0 +1,72 @@
+"""Updated Character Limits for Escaped Values in Requests and Users Tables
+
+Revision ID: a7635f23e449
+Revises: a9d6b5037034
+Create Date: 2022-01-04 21:18:31.060303
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'a7635f23e449'
+down_revision = 'a9d6b5037034'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column('requests', 'description',
+               existing_type=sa.VARCHAR(length=5000),
+               type_=sa.String(length=10240),
+               existing_nullable=True)
+    op.alter_column('requests', 'title',
+               existing_type=sa.VARCHAR(length=90),
+               type_=sa.String(length=256),
+               existing_nullable=True)
+    op.alter_column('users', 'first_name',
+               existing_type=sa.VARCHAR(length=32),
+               type_=sa.String(length=128),
+               existing_nullable=False)
+    op.alter_column('users', 'last_name',
+               existing_type=sa.VARCHAR(length=64),
+               type_=sa.String(length=128),
+               existing_nullable=False)
+    op.alter_column('users', 'organization',
+               existing_type=sa.VARCHAR(length=128),
+               type_=sa.String(length=256),
+               existing_nullable=True)
+    op.alter_column('users', 'title',
+               existing_type=sa.VARCHAR(length=64),
+               type_=sa.String(length=256),
+               existing_nullable=True)
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column('users', 'title',
+               existing_type=sa.String(length=256),
+               type_=sa.VARCHAR(length=64),
+               existing_nullable=True)
+    op.alter_column('users', 'organization',
+               existing_type=sa.String(length=256),
+               type_=sa.VARCHAR(length=128),
+               existing_nullable=True)
+    op.alter_column('users', 'last_name',
+               existing_type=sa.String(length=128),
+               type_=sa.VARCHAR(length=64),
+               existing_nullable=False)
+    op.alter_column('users', 'first_name',
+               existing_type=sa.String(length=128),
+               type_=sa.VARCHAR(length=32),
+               existing_nullable=False)
+    op.alter_column('requests', 'title',
+               existing_type=sa.String(length=256),
+               type_=sa.VARCHAR(length=90),
+               existing_nullable=True)
+    op.alter_column('requests', 'description',
+               existing_type=sa.String(length=10240),
+               type_=sa.VARCHAR(length=5000),
+               existing_nullable=True)
+    # ### end Alembic commands ###