Adds a new xpack.reporting.csv.escapeFormulaValues boolean to auto-es…

…cape potentially bad cells
elastic · Apr 15, 2020 · 2ab046e · 2ab046e
1 parent e44cf28
commit 2ab046e
Show file tree

Hide file tree

Showing 10 changed files with 94 additions and 6 deletions.
diff --git a/x-pack/legacy/plugins/reporting/common/constants.ts b/x-pack/legacy/plugins/reporting/common/constants.ts
@@ -20,6 +20,7 @@ export const API_GENERATE_IMMEDIATE = `${API_BASE_URL_V1}/generate/immediate/csv
 export const CONTENT_TYPE_CSV = 'text/csv';
 export const CSV_REPORTING_ACTION = 'downloadCsvReport';
 export const CSV_BOM_CHARS = '\ufeff';
+export const CSV_FORMULA_CHARS = ['=', '+', '-', '@'];
 
 export const WHITELISTED_JOB_CONTENT_TYPES = [
   'application/json',

diff --git a/x-pack/legacy/plugins/reporting/config.ts b/x-pack/legacy/plugins/reporting/config.ts
@@ -138,6 +138,7 @@ export async function config(Joi: any) {
       useByteOrderMarkEncoding: Joi.boolean().default(false),
       checkForFormulas: Joi.boolean().default(true),
       enablePanelActionDownload: Joi.boolean().default(true),
+      escapeFormulaValues: Joi.boolean().default(false),
       maxSizeBytes: Joi.number()
         .integer()
         .default(1024 * 1024 * 10), // bytes in a kB * kB in a mB * 10

diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.test.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.test.ts
@@ -446,6 +446,50 @@ describe('CSV Execute Job', function() {
     });
   });
 
+  describe('Formula values', () => {
+    it('escapes values with formulas', async () => {
+      configGetStub.withArgs('csv', 'escapeFormulaValues').returns(true);
+      callAsCurrentUserStub.onFirstCall().returns({
+        hits: {
+          hits: [{ _source: { one: `=cmd|' /C calc'!A0`, two: 'bar' } }],
+        },
+        _scroll_id: 'scrollId',
+      });
+
+      const executeJob = await executeJobFactory(mockReportingPlugin, mockLogger);
+      const jobParams = getJobDocPayload({
+        headers: encryptedHeaders,
+        fields: ['one', 'two'],
+        conflictedTypesFields: [],
+        searchRequest: { index: null, body: null },
+      });
+      const { content } = await executeJob('job123', jobParams, cancellationToken);
+
+      expect(content).toEqual("one,two\n\"'=cmd|' /C calc'!A0\",bar\n");
+    });
+
+    it('does not escapes values with formulas', async () => {
+      configGetStub.withArgs('csv', 'escapeFormulaValues').returns(false);
+      callAsCurrentUserStub.onFirstCall().returns({
+        hits: {
+          hits: [{ _source: { one: `=cmd|' /C calc'!A0`, two: 'bar' } }],
+        },
+        _scroll_id: 'scrollId',
+      });
+
+      const executeJob = await executeJobFactory(mockReportingPlugin, mockLogger);
+      const jobParams = getJobDocPayload({
+        headers: encryptedHeaders,
+        fields: ['one', 'two'],
+        conflictedTypesFields: [],
+        searchRequest: { index: null, body: null },
+      });
+      const { content } = await executeJob('job123', jobParams, cancellationToken);
+
+      expect(content).toEqual('one,two\n"=cmd|\' /C calc\'!A0",bar\n');
+    });
+  });
+
   describe('Elasticsearch call errors', function() {
     it('should reject Promise if search call errors out', async function() {
       callAsCurrentUserStub.rejects(new Error());

diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.ts
@@ -136,6 +136,7 @@ export const executeJobFactory: ExecuteJobFactory<ESQueueWorkerExecuteFn<
         checkForFormulas: config.get('csv', 'checkForFormulas'),
         maxSizeBytes: config.get('csv', 'maxSizeBytes'),
         scroll: config.get('csv', 'scroll'),
+        escapeFormulaValues: config.get('csv', 'escapeFormulaValues'),
       },
     });
 

diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.test.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.test.ts
@@ -11,7 +11,7 @@ describe('escapeValue', function() {
   describe('quoteValues is true', function() {
     let escapeValue: (val: string) => string;
     beforeEach(function() {
-      escapeValue = createEscapeValue(true);
+      escapeValue = createEscapeValue(true, false);
     });
 
     it('should escape value with spaces', function() {
@@ -46,12 +46,42 @@ describe('escapeValue', function() {
   describe('quoteValues is false', function() {
     let escapeValue: (val: string) => string;
     beforeEach(function() {
-      escapeValue = createEscapeValue(false);
+      escapeValue = createEscapeValue(false, false);
     });
 
     it('should return the value unescaped', function() {
       const value = '"foo, bar & baz-qux"';
       expect(escapeValue(value)).to.be(value);
     });
   });
+
+  describe('escapeValues', () => {
+    describe('when true', () => {
+      let escapeValue: (val: string) => string;
+      beforeEach(function() {
+        escapeValue = createEscapeValue(true, true);
+      });
+
+      ['@', '+', '-', '='].forEach(badChar => {
+        it(`should escape ${badChar} injection values`, function() {
+          expect(escapeValue(`${badChar}cmd|' /C calc'!A0`)).to.be(
+            `"'${badChar}cmd|' /C calc'!A0"`
+          );
+        });
+      });
+    });
+
+    describe('when false', () => {
+      let escapeValue: (val: string) => string;
+      beforeEach(function() {
+        escapeValue = createEscapeValue(true, false);
+      });
+
+      ['@', '+', '-', '='].forEach(badChar => {
+        it(`should not escape ${badChar} injection values`, function() {
+          expect(escapeValue(`${badChar}cmd|' /C calc'!A0`)).to.be(`"${badChar}cmd|' /C calc'!A0"`);
+        });
+      });
+    });
+  });
 });
diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.ts
@@ -5,15 +5,23 @@
  */
 
 import { RawValue } from './types';
+import { CSV_FORMULA_CHARS } from '../../../../common/constants';
 
 const nonAlphaNumRE = /[^a-zA-Z0-9]/;
 const allDoubleQuoteRE = /"/g;
 
-export function createEscapeValue(quoteValues: boolean): (val: RawValue) => string {
+const valHasFormulas = (val: string) =>
+  CSV_FORMULA_CHARS.some(formulaChar => val.startsWith(formulaChar));
+
+export function createEscapeValue(
+  quoteValues: boolean,
+  escapeFormulas: boolean
+): (val: RawValue) => string {
   return function escapeValue(val: RawValue) {
     if (val && typeof val === 'string') {
-      if (quoteValues && nonAlphaNumRE.test(val)) {
-        return `"${val.replace(allDoubleQuoteRE, '""')}"`;
+      const formulasEscaped = escapeFormulas && valHasFormulas(val) ? "'" + val : val;
+      if (quoteValues && nonAlphaNumRE.test(formulasEscaped)) {
+        return `"${formulasEscaped.replace(allDoubleQuoteRE, '""')}"`;
       }
     }
 

diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.ts b/x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.ts
@@ -26,7 +26,7 @@ export function createGenerateCsv(logger: Logger) {
     cancellationToken,
     settings,
   }: GenerateCsvParams): Promise<SavedSearchGeneratorResult> {
-    const escapeValue = createEscapeValue(settings.quoteValues);
+    const escapeValue = createEscapeValue(settings.quoteValues, settings.escapeFormulaValues);
     const builder = new MaxSizeStringBuilder(settings.maxSizeBytes);
     const header = `${fields.map(escapeValue).join(settings.separator)}\n`;
     if (!builder.tryAppend(header)) {

diff --git a/x-pack/legacy/plugins/reporting/export_types/csv/types.d.ts b/x-pack/legacy/plugins/reporting/export_types/csv/types.d.ts
@@ -109,5 +109,6 @@ export interface GenerateCsvParams {
     maxSizeBytes: number;
     scroll: ScrollConfig;
     checkForFormulas?: boolean;
+    escapeFormulaValues: boolean;
   };
 }
diff --git a/...acy/plugins/reporting/export_types/csv_from_savedobject/server/lib/generate_csv_search.ts b/...acy/plugins/reporting/export_types/csv_from_savedobject/server/lib/generate_csv_search.ts
@@ -173,6 +173,7 @@ export async function generateCsvSearch(
       ...uiSettings,
       maxSizeBytes: config.get('csv', 'maxSizeBytes'),
       scroll: config.get('csv', 'scroll'),
+      escapeFormulaValues: config.get('csv', 'escapeFormulaValues'),
       timezone,
     },
   };

diff --git a/x-pack/legacy/plugins/reporting/server/config/index.ts b/x-pack/legacy/plugins/reporting/server/config/index.ts
@@ -109,6 +109,7 @@ export interface ReportingConfigType {
     checkForFormulas: boolean;
     maxSizeBytes: number;
     useByteOrderMarkEncoding: boolean;
+    escapeFormulaValues: boolean;
   };
   encryptionKey: string;
   kibanaServer: any;