From 3ccb14fc51c93a749f5a6d4e06ec99047832c443 Mon Sep 17 00:00:00 2001
From: Gidi Meir Morris <github@gidi.io>
Date: Wed, 24 Jun 2020 10:26:48 +0100
Subject: [PATCH] prevent parens and whitespace in consumer or alerttypeid

---
 .../authorization/alerts_authorization.test.ts | 14 +++++++++++---
 .../authorization/alerts_authorization.ts      | 18 +++++++++++-------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts
index 507fd8f0304a27..c0c4c9caa08445 100644
--- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts
+++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts
@@ -816,12 +816,20 @@ describe('ensureFieldIsSafeForQuery', () => {
       `expected id not to include invalid character: <=`
     );
 
-    expect(() => ensureFieldIsSafeForQuery('id', '<"" or >=""')).toThrowError(
-      `expected id not to include invalid characters: <, >=`
+    expect(() => ensureFieldIsSafeForQuery('id', '>=""')).toThrowError(
+      `expected id not to include invalid character: >=`
     );
 
     expect(() => ensureFieldIsSafeForQuery('id', '1 or alertid:123')).toThrowError(
-      `expected id not to include invalid character: :`
+      `expected id not to include whitespace and invalid character: :`
+    );
+
+    expect(() => ensureFieldIsSafeForQuery('id', ') or alertid:123')).toThrowError(
+      `expected id not to include whitespace and invalid characters: ), :`
+    );
+
+    expect(() => ensureFieldIsSafeForQuery('id', 'some space')).toThrowError(
+      `expected id not to include whitespace`
     );
   });
 
diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts
index a0362b1d7a312d..5a43e3050ae93a 100644
--- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts
+++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts
@@ -5,7 +5,7 @@
  */
 
 import Boom from 'boom';
-import { pluck, mapValues } from 'lodash';
+import { pluck, mapValues, remove } from 'lodash';
 import { KibanaRequest } from 'src/core/server';
 import { ALERTS_FEATURE_ID } from '../../common';
 import { AlertTypeRegistry } from '../types';
@@ -269,13 +269,17 @@ export class AlertsAuthorization {
 }
 
 export function ensureFieldIsSafeForQuery(field: string, value: string): boolean {
-  const invalid = value.match(/[>=<\*:]+/g);
+  const invalid = value.match(/([>=<\*:()]+|\s+)/g);
   if (invalid) {
-    throw new Error(
-      `expected ${field} not to include invalid character${
-        invalid.length > 1 ? `s` : ``
-      }: ${invalid?.join(`, `)}`
-    );
+    const whitespace = remove(invalid, (chars) => chars.trim().length === 0);
+    const errors = [];
+    if (whitespace.length) {
+      errors.push(`whitespace`);
+    }
+    if (invalid.length) {
+      errors.push(`invalid character${invalid.length > 1 ? `s` : ``}: ${invalid?.join(`, `)}`);
+    }
+    throw new Error(`expected ${field} not to include ${errors.join(' and ')}`);
   }
   return true;
 }