From 1b791d55839ebf434e104cc9936ccb8c29019231 Mon Sep 17 00:00:00 2001 From: John Molomby Date: Fri, 6 Oct 2017 16:36:06 +1100 Subject: [PATCH] Add some functionality to prevent macro injection via CSV export --- admin/server/api/download.js | 6 ++++++ lib/list/getCSVData.js | 6 ++++++ lib/security/escapeValueForExcel | 17 +++++++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 lib/security/escapeValueForExcel diff --git a/admin/server/api/download.js b/admin/server/api/download.js index 59efc4918e2..989ad101003 100644 --- a/admin/server/api/download.js +++ b/admin/server/api/download.js @@ -8,6 +8,7 @@ supports more features at the moment (custom .toCSV method on lists, etc) var _ = require('lodash'); var async = require('async'); var moment = require('moment'); +var escapeValueForExcel = require('../security/escapeValueForExcel'); var FN_ARGS = /^function\s*[^\(]*\(\s*([^\)]*)\)/m; @@ -64,6 +65,11 @@ module.exports = function (req, res) { } }); + // Prevent CSV macro injection + _.forOwn(rowData, (value, prop) => { + rowData[prop] = escapeValueForExcel(value); + }); + return rowData; }; diff --git a/lib/list/getCSVData.js b/lib/list/getCSVData.js index c76d7664034..f6103f8e72a 100644 --- a/lib/list/getCSVData.js +++ b/lib/list/getCSVData.js @@ -1,5 +1,6 @@ var _ = require('lodash'); var listToArray = require('list-to-array'); +var escapeValueForExcel = require('../security/escapeValueForExcel'); /** * Applies option field transforms to get the CSV value for a field @@ -110,6 +111,11 @@ function getCSVData (item, options) { } }); + // Prevent CSV macro injection + _.forOwn(rtn, (value, prop) => { + rtn[prop] = escapeValueForExcel(value); + }); + return rtn; } diff --git a/lib/security/escapeValueForExcel b/lib/security/escapeValueForExcel new file mode 100644 index 00000000000..e93beea6f9e --- /dev/null +++ b/lib/security/escapeValueForExcel @@ -0,0 +1,17 @@ +// This prevents macro injection for values included in a CSV by prepending some values with a space +// Most resources suggest prepending an apostrophe but Excel has a nasty habit of stripping leading apostrophes out when saving back to CSV files +// Spaces have the benifits of.. +// - Being maintained by Excel +// - Not interfearing with number parsing (ie. " -100" is still automatically formatted as a number where as "'-100" isn't) +// - Probably easier to deal with if the CSV file is consumed by another tool (ie. the values can be trimmed before consumption) + +const formulaTriggers = ['+', '-', '=', '@']; + +function escapeValueForExcel (value) { + if (formulaTriggers.indexOf(value.toString().slice(0, 1)) > 0) { + return ' ' + value; + } + return value; +}; + +module.exports = escapeValueForExcel;