[Exceptions][Value Lists] Add file type and size constraints to value…

… list uploads (#8507) (#176074) ## Summary Addresses elastic/security-team#8507 With these changes we address the issue where users can upload any file to be imported as a value list. The restrictions are: * Users should be limited to uploading .txt or .csv. All other file types should return a 415. * Users should be limited to uploading files of 9K bytes size. Files larger than that should return a 413. ### Checklist Delete any items that are not applicable to this PR. - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ESS 97 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5019) - [Serverless 97 times](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5020)
elastic · Feb 7, 2024 · c38410a · c38410a
1 parent e90c209
commit c38410a
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 8 deletions.
diff --git a/x-pack/plugins/lists/server/routes/list/import_list_item_route.ts b/x-pack/plugins/lists/server/routes/list/import_list_item_route.ts
@@ -5,6 +5,8 @@
  * 2.0.
  */
 
+import { extname } from 'path';
+
 import { schema } from '@kbn/config-schema';
 import { validate } from '@kbn/securitysolution-io-ts-utils';
 import { transformError } from '@kbn/securitysolution-es-utils';
@@ -17,6 +19,8 @@ import { buildRouteValidation, buildSiemResponse } from '../utils';
 import { createStreamFromBuffer } from '../utils/create_stream_from_buffer';
 import { getListClient } from '..';
 
+const validFileExtensions = ['.csv', '.txt'];
+
 export const importListItemRoute = (router: ListsPluginRouter, config: ConfigType): void => {
   router.versioned
     .post({
@@ -47,10 +51,29 @@ export const importListItemRoute = (router: ListsPluginRouter, config: ConfigTyp
       async (context, request, response) => {
         const siemResponse = buildSiemResponse(response);
         try {
-          const stream = createStreamFromBuffer(request.body);
           const { deserializer, list_id: listId, serializer, type } = request.query;
           const lists = await getListClient(context);
 
+          const filename = await lists.getImportFilename({
+            stream: createStreamFromBuffer(request.body),
+          });
+          if (!filename) {
+            return siemResponse.error({
+              body: 'To import a list item, the file name must be specified',
+              statusCode: 400,
+            });
+          }
+          const fileExtension = extname(filename).toLowerCase();
+          if (!validFileExtensions.includes(fileExtension)) {
+            return siemResponse.error({
+              body: `Unsupported media type. File must be one of the following types: [${validFileExtensions.join(
+                ', '
+              )}]`,
+              statusCode: 415,
+            });
+          }
+
+          const stream = createStreamFromBuffer(request.body);
           const listDataExists = await lists.getListDataStreamExists();
           if (!listDataExists) {
             const listIndexExists = await lists.getListIndexExists();

diff --git a/x-pack/plugins/lists/server/services/lists/list_client.ts b/x-pack/plugins/lists/server/services/lists/list_client.ts
@@ -37,6 +37,7 @@ import type {
 
 import type { ConfigType } from '../../config';
 import {
+  BufferLines,
   createListItem,
   deleteListItem,
   deleteListItemByValue,
@@ -69,6 +70,7 @@ import type {
   FindAllListItemsOptions,
   FindListItemOptions,
   FindListOptions,
+  GetImportFilename,
   GetListItemByValueOptions,
   GetListItemOptions,
   GetListItemsByValueOptions,
@@ -715,6 +717,33 @@ export class ListClient {
     });
   };
 
+  /**
+   * Gets the filename of the imported file
+   * @param options
+   * @param options.stream The stream to pull the import from
+   * @returns
+   */
+  public getImportFilename = ({ stream }: GetImportFilename): Promise<string | undefined> => {
+    return new Promise<string | undefined>((resolve, reject) => {
+      const { config } = this;
+      const readBuffer = new BufferLines({ bufferSize: config.importBufferSize, input: stream });
+      let fileName: string | undefined;
+      readBuffer.on('fileName', async (fileNameEmitted: string) => {
+        try {
+          readBuffer.pause();
+          fileName = decodeURIComponent(fileNameEmitted);
+          readBuffer.resume();
+        } catch (err) {
+          reject(err);
+        }
+      });
+
+      readBuffer.on('close', () => {
+        resolve(fileName);
+      });
+    });
+  };
+
   /**
    * Imports list items to a stream. If the list already exists, this will append the list items to the existing list.
    * If the list does not exist, this will auto-create the list and then add the items to that list.

diff --git a/x-pack/plugins/lists/server/services/lists/list_client_types.ts b/x-pack/plugins/lists/server/services/lists/list_client_types.ts
@@ -335,3 +335,12 @@ export interface SearchListItemByValuesOptions {
   /** The value to search for list items based off. */
   value: unknown[];
 }
+
+/**
+ * ListClient.getImportFilename
+ * {@link ListClient.getImportFilename}
+ */
+export interface GetImportFilename {
+  /** The stream to pull the import from */
+  stream: Readable;
+}
diff --git a/.../exceptions/workflows/trial_license_complete_tier/role_based_rule_exceptions_workflows.ts b/.../exceptions/workflows/trial_license_complete_tier/role_based_rule_exceptions_workflows.ts
@@ -803,7 +803,7 @@ export default ({ getService }: FtrProviderContext) => {
           });
 
           it('generates no alerts when a value list exception is added for a query rule', async () => {
-            const valueListId = 'value-list-id';
+            const valueListId = 'value-list-id.txt';
             await importFile(supertest, log, 'keyword', ['suricata-sensor-amsterdam'], valueListId);
             const rule: QueryRuleCreateProps = {
               name: 'Simple Rule Query',
@@ -835,7 +835,7 @@ export default ({ getService }: FtrProviderContext) => {
           });
 
           it('generates no alerts when a value list exception is added for a threat match rule', async () => {
-            const valueListId = 'value-list-id';
+            const valueListId = 'value-list-id.txt';
             await importFile(supertest, log, 'keyword', ['zeek-sensor-amsterdam'], valueListId);
             const rule: ThreatMatchRuleCreateProps = {
               description: 'Detecting root and admin users',
@@ -883,7 +883,7 @@ export default ({ getService }: FtrProviderContext) => {
           });
 
           it('generates no alerts when a value list exception is added for a threshold rule', async () => {
-            const valueListId = 'value-list-id';
+            const valueListId = 'value-list-id.txt';
             await importFile(supertest, log, 'keyword', ['zeek-sensor-amsterdam'], valueListId);
             const rule: ThresholdRuleCreateProps = {
               description: 'Detecting root and admin users',
@@ -920,7 +920,7 @@ export default ({ getService }: FtrProviderContext) => {
           });
 
           it('generates no alerts when a value list exception is added for an EQL rule', async () => {
-            const valueListId = 'value-list-id';
+            const valueListId = 'value-list-id.txt';
             await importFile(supertest, log, 'keyword', ['zeek-sensor-amsterdam'], valueListId);
             const rule: EqlRuleCreateProps = {
               ...getEqlRuleForAlertTesting(['auditbeat-*']),
@@ -944,7 +944,7 @@ export default ({ getService }: FtrProviderContext) => {
             expect(alertsOpen.hits.hits.length).toEqual(0);
           });
           it('should Not allow deleting value list when there are references and ignoreReferences is false', async () => {
-            const valueListId = 'value-list-id';
+            const valueListId = 'value-list-id.txt';
             await importFile(supertest, log, 'keyword', ['suricata-sensor-amsterdam'], valueListId);
             const rule: QueryRuleCreateProps = {
               ...getSimpleRule(),

diff --git a/...ngine/exceptions/workflows/trial_license_complete_tier/rule_exception_synchronizations.ts b/...ngine/exceptions/workflows/trial_license_complete_tier/rule_exception_synchronizations.ts
@@ -107,7 +107,7 @@ export default ({ getService }: FtrProviderContext) => {
     it('should Not allow editing an Exception with deleted ValueList', async () => {
       await createListsIndex(supertest, log);
 
-      const valueListId = 'value-list-id';
+      const valueListId = 'value-list-id.txt';
       await importFile(supertest, log, 'keyword', ['suricata-sensor-amsterdam'], valueListId);
       const rule: QueryRuleCreateProps = {
         ...getSimpleRule(),

diff --git a/...gine/rule_execution_logic/trial_license_complete_tier/execution_logic/machine_learning.ts b/...gine/rule_execution_logic/trial_license_complete_tier/execution_logic/machine_learning.ts
@@ -229,7 +229,7 @@ export default ({ getService }: FtrProviderContext) => {
       });
 
       it('generates no alerts when a value list exception is added for an ML rule', async () => {
-        const valueListId = 'value-list-id';
+        const valueListId = 'value-list-id.txt';
         await importFile(supertest, log, 'keyword', ['mothra'], valueListId);
         const { previewId } = await previewRuleWithExceptionEntries({
           supertest,

diff --git a/...ts_and_exception_lists/lists_items/trial_license_complete_tier/items/import_list_items.ts b/...ts_and_exception_lists/lists_items/trial_license_complete_tier/items/import_list_items.ts
@@ -54,6 +54,39 @@ export default ({ getService }: FtrProviderContext): void => {
         await deleteListsIndex(supertest, log);
       });
 
+      it('should not import a list item if the imported file is not .txt or .csv', async () => {
+        const { body } = await supertest
+          .post(`${LIST_ITEM_URL}/_import?type=ip`)
+          .set('kbn-xsrf', 'true')
+          .attach('file', getImportListItemAsBuffer(['127.0.0.1', '127.0.0.2']), 'list_items.pdf')
+          .expect('Content-Type', 'application/json; charset=utf-8')
+          .expect(415);
+
+        expect(body).to.eql({
+          status_code: 415,
+          message: 'Unsupported media type. File must be one of the following types: [.csv, .txt]',
+        });
+      });
+
+      it('should not import a list item if the imported file exceed the file size limit', async () => {
+        const { body } = await supertest
+          .post(`${LIST_ITEM_URL}/_import?type=ip`)
+          .set('kbn-xsrf', 'true')
+          .attach(
+            'file',
+            getImportListItemAsBuffer(Array(1000000).fill('127.0.0.1')),
+            'list_items.txt'
+          )
+          .expect('Content-Type', 'application/json; charset=utf-8')
+          .expect(413);
+
+        expect(body).to.eql({
+          statusCode: 413,
+          error: 'Request Entity Too Large',
+          message: 'Payload content length greater than maximum allowed: 9000000',
+        });
+      });
+
       it('should set the response content types to be expected when importing two items', async () => {
         await supertest
           .post(`${LIST_ITEM_URL}/_import?type=ip`)