From 767859cb0cc741d9258287b04b3360981bc36a62 Mon Sep 17 00:00:00 2001 From: Marshall Main <55718608+marshallmain@users.noreply.github.com> Date: Wed, 3 Mar 2021 14:15:38 -0800 Subject: [PATCH] [Security Solution][Lists] Escape quotes in list ids and quote the id in KQL query (#93176) (#93502) * Escape quotes in list ids and quote the id in KQL query * Remove decodeURIComponent because too many KQL queries don't handle quotes * Add quotes to user supplied IDs for other KQL queries Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../lists/server/routes/delete_list_route.ts | 3 ++- .../find_exception_list_items.test.ts | 25 +++++++++++++------ .../find_exception_list_items.ts | 7 ++++-- .../server/services/utils/escape_query.ts | 10 ++++++++ .../services/utils/get_query_filter.test.ts | 4 +-- .../server/services/utils/get_query_filter.ts | 7 +++++- .../security_and_spaces/tests/delete_lists.ts | 22 ++++++++++++++++ 7 files changed, 65 insertions(+), 13 deletions(-) create mode 100644 x-pack/plugins/lists/server/services/utils/escape_query.ts diff --git a/x-pack/plugins/lists/server/routes/delete_list_route.ts b/x-pack/plugins/lists/server/routes/delete_list_route.ts index 4732b25dbf5e7f..3e9b76a1b330a9 100644 --- a/x-pack/plugins/lists/server/routes/delete_list_route.ts +++ b/x-pack/plugins/lists/server/routes/delete_list_route.ts @@ -19,6 +19,7 @@ import { } from '../../common/schemas'; import { getSavedObjectType } from '../services/exception_lists/utils'; import { ExceptionListClient } from '../services/exception_lists/exception_list_client'; +import { escapeQuotes } from '../services/utils/escape_query'; import { getExceptionListClient, getListClient } from '.'; @@ -142,7 +143,7 @@ const getReferencedExceptionLists = async ( (item) => `${getSavedObjectType({ namespaceType: item.namespace_type, - })}.attributes.list_id: ${item.list_id}` + })}.attributes.list_id: "${escapeQuotes(item.list_id)}"` ) .join(' OR '); return exceptionLists.findExceptionList({ diff --git a/x-pack/plugins/lists/server/services/exception_lists/find_exception_list_items.test.ts b/x-pack/plugins/lists/server/services/exception_lists/find_exception_list_items.test.ts index 0d3dd2d9b65c38..3a2b12c3589175 100644 --- a/x-pack/plugins/lists/server/services/exception_lists/find_exception_list_items.test.ts +++ b/x-pack/plugins/lists/server/services/exception_lists/find_exception_list_items.test.ts @@ -18,7 +18,18 @@ describe('find_exception_list_items', () => { savedObjectType: ['exception-list'], }); expect(filter).toEqual( - '(exception-list.attributes.list_type: item AND exception-list.attributes.list_id: some-list-id)' + '(exception-list.attributes.list_type: item AND exception-list.attributes.list_id: "some-list-id")' + ); + }); + + test('It should create a filter escaping quotes in list ids', () => { + const filter = getExceptionListsItemFilter({ + filter: [], + listId: ['list-id-"-with-quote'], + savedObjectType: ['exception-list'], + }); + expect(filter).toEqual( + '(exception-list.attributes.list_type: item AND exception-list.attributes.list_id: "list-id-\\"-with-quote")' ); }); @@ -29,7 +40,7 @@ describe('find_exception_list_items', () => { savedObjectType: ['exception-list'], }); expect(filter).toEqual( - '((exception-list.attributes.list_type: item AND exception-list.attributes.list_id: some-list-id) AND exception-list.attributes.name: "Sample Endpoint Exception List")' + '((exception-list.attributes.list_type: item AND exception-list.attributes.list_id: "some-list-id") AND exception-list.attributes.name: "Sample Endpoint Exception List")' ); }); @@ -40,7 +51,7 @@ describe('find_exception_list_items', () => { savedObjectType: ['exception-list', 'exception-list-agnostic'], }); expect(filter).toEqual( - '(exception-list.attributes.list_type: item AND exception-list.attributes.list_id: list-1) OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: list-2)' + '(exception-list.attributes.list_type: item AND exception-list.attributes.list_id: "list-1") OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "list-2")' ); }); @@ -51,7 +62,7 @@ describe('find_exception_list_items', () => { savedObjectType: ['exception-list', 'exception-list-agnostic'], }); expect(filter).toEqual( - '((exception-list.attributes.list_type: item AND exception-list.attributes.list_id: list-1) AND exception-list.attributes.name: "Sample Endpoint Exception List") OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: list-2)' + '((exception-list.attributes.list_type: item AND exception-list.attributes.list_id: "list-1") AND exception-list.attributes.name: "Sample Endpoint Exception List") OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "list-2")' ); }); @@ -62,7 +73,7 @@ describe('find_exception_list_items', () => { savedObjectType: ['exception-list', 'exception-list-agnostic', 'exception-list-agnostic'], }); expect(filter).toEqual( - '(exception-list.attributes.list_type: item AND exception-list.attributes.list_id: list-1) OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: list-2) OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: list-3)' + '(exception-list.attributes.list_type: item AND exception-list.attributes.list_id: "list-1") OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "list-2") OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "list-3")' ); }); @@ -73,7 +84,7 @@ describe('find_exception_list_items', () => { savedObjectType: ['exception-list', 'exception-list-agnostic', 'exception-list-agnostic'], }); expect(filter).toEqual( - '((exception-list.attributes.list_type: item AND exception-list.attributes.list_id: list-1) AND exception-list.attributes.name: "Sample Endpoint Exception List") OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: list-2) OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: list-3)' + '((exception-list.attributes.list_type: item AND exception-list.attributes.list_id: "list-1") AND exception-list.attributes.name: "Sample Endpoint Exception List") OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "list-2") OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "list-3")' ); }); @@ -88,7 +99,7 @@ describe('find_exception_list_items', () => { savedObjectType: ['exception-list', 'exception-list-agnostic', 'exception-list-agnostic'], }); expect(filter).toEqual( - '((exception-list.attributes.list_type: item AND exception-list.attributes.list_id: list-1) AND exception-list.attributes.name: "Sample Endpoint Exception List 1") OR ((exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: list-2) AND exception-list.attributes.name: "Sample Endpoint Exception List 2") OR ((exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: list-3) AND exception-list.attributes.name: "Sample Endpoint Exception List 3")' + '((exception-list.attributes.list_type: item AND exception-list.attributes.list_id: "list-1") AND exception-list.attributes.name: "Sample Endpoint Exception List 1") OR ((exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "list-2") AND exception-list.attributes.name: "Sample Endpoint Exception List 2") OR ((exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.list_id: "list-3") AND exception-list.attributes.name: "Sample Endpoint Exception List 3")' ); }); }); diff --git a/x-pack/plugins/lists/server/services/exception_lists/find_exception_list_items.ts b/x-pack/plugins/lists/server/services/exception_lists/find_exception_list_items.ts index cc84314eaa7a02..155408dafc79dd 100644 --- a/x-pack/plugins/lists/server/services/exception_lists/find_exception_list_items.ts +++ b/x-pack/plugins/lists/server/services/exception_lists/find_exception_list_items.ts @@ -24,6 +24,7 @@ import { SortFieldOrUndefined, SortOrderOrUndefined, } from '../../../common/schemas'; +import { escapeQuotes } from '../utils/escape_query'; import { getSavedObjectTypes, transformSavedObjectsToFoundExceptionListItem } from './utils'; import { getExceptionList } from './get_exception_list'; @@ -89,7 +90,8 @@ export const getExceptionListsItemFilter = ({ savedObjectType: SavedObjectType[]; }): string => { return listId.reduce((accum, singleListId, index) => { - const listItemAppend = `(${savedObjectType[index]}.attributes.list_type: item AND ${savedObjectType[index]}.attributes.list_id: ${singleListId})`; + const escapedListId = escapeQuotes(singleListId); + const listItemAppend = `(${savedObjectType[index]}.attributes.list_type: item AND ${savedObjectType[index]}.attributes.list_id: "${escapedListId}")`; const listItemAppendWithFilter = filter[index] != null ? `(${listItemAppend} AND ${filter[index]})` : listItemAppend; if (accum === '') { @@ -117,8 +119,9 @@ export const findValueListExceptionListItems = async ({ sortField, sortOrder, }: FindValueListExceptionListsItems): Promise => { + const escapedValueListId = escapeQuotes(valueListId); const savedObjectsFindResponse = await savedObjectsClient.find({ - filter: `(exception-list.attributes.list_type: item AND exception-list.attributes.entries.list.id:${valueListId}) OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.entries.list.id:${valueListId}) `, + filter: `(exception-list.attributes.list_type: item AND exception-list.attributes.entries.list.id:"${escapedValueListId}") OR (exception-list-agnostic.attributes.list_type: item AND exception-list-agnostic.attributes.entries.list.id:"${escapedValueListId}") `, page, perPage, sortField, diff --git a/x-pack/plugins/lists/server/services/utils/escape_query.ts b/x-pack/plugins/lists/server/services/utils/escape_query.ts new file mode 100644 index 00000000000000..f654b8a2b9ebe0 --- /dev/null +++ b/x-pack/plugins/lists/server/services/utils/escape_query.ts @@ -0,0 +1,10 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export const escapeQuotes = (str: string): string => { + return str.replace(/[\\"]/g, '\\$&'); +}; diff --git a/x-pack/plugins/lists/server/services/utils/get_query_filter.test.ts b/x-pack/plugins/lists/server/services/utils/get_query_filter.test.ts index d189012aec0e12..0f6cc171bc04c0 100644 --- a/x-pack/plugins/lists/server/services/utils/get_query_filter.test.ts +++ b/x-pack/plugins/lists/server/services/utils/get_query_filter.test.ts @@ -46,7 +46,7 @@ describe('get_query_filter', () => { minimum_should_match: 1, should: [ { - match: { + match_phrase: { list_id: 'list-123', }, }, @@ -74,7 +74,7 @@ describe('get_query_filter', () => { minimum_should_match: 1, should: [ { - match: { + match_phrase: { list_id: 'list-123', }, }, diff --git a/x-pack/plugins/lists/server/services/utils/get_query_filter.ts b/x-pack/plugins/lists/server/services/utils/get_query_filter.ts index 5cbad8c284a553..25c8f9880063fb 100644 --- a/x-pack/plugins/lists/server/services/utils/get_query_filter.ts +++ b/x-pack/plugins/lists/server/services/utils/get_query_filter.ts @@ -9,6 +9,8 @@ import { DslQuery, EsQueryConfig } from 'src/plugins/data/common'; import { Filter, Query, esQuery } from '../../../../../../src/plugins/data/server'; +import { escapeQuotes } from './escape_query'; + export interface GetQueryFilterOptions { filter: string; } @@ -41,7 +43,10 @@ export const getQueryFilterWithListId = ({ filter, listId, }: GetQueryFilterWithListIdOptions): GetQueryFilterReturn => { + const escapedListId = escapeQuotes(listId); const filterWithListId = - filter.trim() !== '' ? `list_id: ${listId} AND (${filter})` : `list_id: ${listId}`; + filter.trim() !== '' + ? `list_id: "${escapedListId}" AND (${filter})` + : `list_id: "${escapedListId}"`; return getQueryFilter({ filter: filterWithListId }); }; diff --git a/x-pack/test/lists_api_integration/security_and_spaces/tests/delete_lists.ts b/x-pack/test/lists_api_integration/security_and_spaces/tests/delete_lists.ts index adba0a2f626e19..4ce3c7f0e5661d 100644 --- a/x-pack/test/lists_api_integration/security_and_spaces/tests/delete_lists.ts +++ b/x-pack/test/lists_api_integration/security_and_spaces/tests/delete_lists.ts @@ -63,6 +63,28 @@ export default ({ getService }: FtrProviderContext) => { expect(bodyToCompare).to.eql(getListResponseMockWithoutAutoGeneratedValues()); }); + it('should delete a single list with a list id containing non-alphanumeric characters', async () => { + // create a list + const id = `some""-list-id"(1)`; + await supertest + .post(LIST_URL) + .set('kbn-xsrf', 'true') + .send({ + ...getCreateMinimalListSchemaMock(), + id, + }) + .expect(200); + + // delete the list by its list id + const { body } = await supertest + .delete(`${LIST_URL}?id=${id}`) + .set('kbn-xsrf', 'true') + .expect(200); + + const bodyToCompare = removeListServerGeneratedProperties(body); + expect(bodyToCompare).to.eql(getListResponseMockWithoutAutoGeneratedValues()); + }); + it('should delete a single list using an auto generated id', async () => { // add a list const { body: bodyWithCreatedList } = await supertest