From d3de8c4ffd45cca2ed9816e34e179c5641db5dc6 Mon Sep 17 00:00:00 2001 From: YANG QIA <2013xile@gmail.com> Date: Thu, 19 Dec 2024 13:16:06 +0800 Subject: [PATCH] fix(sql-collection): block dangerous keywords (#5913) --- .../src/server/__tests__/actions.test.ts | 20 +++++++++ .../src/server/plugin.ts | 16 +++++++ .../src/server/resources/sql.ts | 10 +++-- .../plugin-collection-sql/src/server/utils.ts | 43 +++++++++++++++++++ 4 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 packages/plugins/@nocobase/plugin-collection-sql/src/server/utils.ts diff --git a/packages/plugins/@nocobase/plugin-collection-sql/src/server/__tests__/actions.test.ts b/packages/plugins/@nocobase/plugin-collection-sql/src/server/__tests__/actions.test.ts index 1dcdca9809..472c01d77c 100644 --- a/packages/plugins/@nocobase/plugin-collection-sql/src/server/__tests__/actions.test.ts +++ b/packages/plugins/@nocobase/plugin-collection-sql/src/server/__tests__/actions.test.ts @@ -42,6 +42,14 @@ describe('sql collection', () => { }); expect(res.status).toBe(400); expect(res.body.errors[0].message).toMatch('Only supports SELECT statements or WITH clauses'); + + res = await agent.resource('sqlCollection').execute({ + values: { + sql: "select pg_read_file('/etc/passwd');", + }, + }); + expect(res.status).toBe(400); + expect(res.body.errors[0].message).toMatch('SQL statements contain dangerous keywords'); }); it('sqlCollection:execute', async () => { @@ -244,4 +252,16 @@ describe('sql collection', () => { const loadedFields2 = db.getCollection('sqlCollection').fields; expect(loadedFields2.size).toBe(1); }); + + it('should check sql when creating', async () => { + const res = await agent.resource('collections').create({ + values: { + name: 'sqlCollection', + sql: "select pg_read_file('/etc/passwd');", + template: 'sql', + }, + }); + expect(res.status).toBe(400); + expect(res.body.errors[0].message).toMatch('SQL statements contain dangerous keywords'); + }); }); diff --git a/packages/plugins/@nocobase/plugin-collection-sql/src/server/plugin.ts b/packages/plugins/@nocobase/plugin-collection-sql/src/server/plugin.ts index 1cea609a90..adcf675a61 100644 --- a/packages/plugins/@nocobase/plugin-collection-sql/src/server/plugin.ts +++ b/packages/plugins/@nocobase/plugin-collection-sql/src/server/plugin.ts @@ -11,6 +11,7 @@ import { Plugin } from '@nocobase/server'; import { Collection } from '@nocobase/database'; import { SQLCollection } from './sql-collection'; import sqlResourcer from './resources/sql'; +import { checkSQL } from './utils'; export class PluginCollectionSQLServer extends Plugin { async beforeLoad() { @@ -34,6 +35,21 @@ export class PluginCollectionSQLServer extends Plugin { name: `pm.data-source-manager.collection-sql `, actions: ['sqlCollection:*'], }); + + this.app.resourceManager.use(async (ctx, next) => { + const { resourceName, actionName } = ctx.action; + if (resourceName === 'collections' && actionName === 'create') { + const { sql } = ctx.action.params.values || {}; + if (sql) { + try { + checkSQL(sql); + } catch (e) { + ctx.throw(400, ctx.t(e.message)); + } + } + } + return next(); + }); } } diff --git a/packages/plugins/@nocobase/plugin-collection-sql/src/server/resources/sql.ts b/packages/plugins/@nocobase/plugin-collection-sql/src/server/resources/sql.ts index 9361533b5e..b366200724 100644 --- a/packages/plugins/@nocobase/plugin-collection-sql/src/server/resources/sql.ts +++ b/packages/plugins/@nocobase/plugin-collection-sql/src/server/resources/sql.ts @@ -9,6 +9,7 @@ import { Context, Next } from '@nocobase/actions'; import { SQLCollection, SQLModel } from '../sql-collection'; +import { checkSQL } from '../utils'; const updateCollection = async (ctx: Context, transaction: any) => { const { filterByTk, values } = ctx.action.params; @@ -48,13 +49,14 @@ export default { name: 'sqlCollection', actions: { execute: async (ctx: Context, next: Next) => { - let { sql } = ctx.action.params.values || {}; + const { sql } = ctx.action.params.values || {}; if (!sql) { ctx.throw(400, ctx.t('Please enter a SQL statement')); } - sql = sql.trim().split(';').shift(); - if (!/^select/i.test(sql) && !/^with([\s\S]+)select([\s\S]+)/i.test(sql)) { - ctx.throw(400, ctx.t('Only supports SELECT statements or WITH clauses')); + try { + checkSQL(sql); + } catch (e) { + ctx.throw(400, ctx.t(e.message)); } const tmpCollection = new SQLCollection({ name: 'tmp', sql }, { database: ctx.db }); const model = tmpCollection.model as typeof SQLModel; diff --git a/packages/plugins/@nocobase/plugin-collection-sql/src/server/utils.ts b/packages/plugins/@nocobase/plugin-collection-sql/src/server/utils.ts new file mode 100644 index 0000000000..bba38f2cd8 --- /dev/null +++ b/packages/plugins/@nocobase/plugin-collection-sql/src/server/utils.ts @@ -0,0 +1,43 @@ +/** + * This file is part of the NocoBase (R) project. + * Copyright (c) 2020-2024 NocoBase Co., Ltd. + * Authors: NocoBase Team. + * + * This project is dual-licensed under AGPL-3.0 and NocoBase Commercial License. + * For more information, please refer to: https://www.nocobase.com/agreement. + */ + +export const checkSQL = (sql: string) => { + const dangerKeywords = [ + // PostgreSQL + 'pg_read_file', + 'pg_read_binary_file', + 'pg_stat_file', + 'pg_ls_dir', + 'pg_logdir_ls', + 'pg_terminate_backend', + 'pg_cancel_backend', + 'current_setting', + 'set_config', + 'pg_reload_conf', + 'pg_sleep', + 'generate_series', + + // MySQL + 'LOAD_FILE', + 'BENCHMARK', + '@@global.', + '@@session.', + + // SQLite + 'sqlite3_load_extension', + 'load_extension', + ]; + sql = sql.trim().split(';').shift(); + if (!/^select/i.test(sql) && !/^with([\s\S]+)select([\s\S]+)/i.test(sql)) { + throw new Error('Only supports SELECT statements or WITH clauses'); + } + if (dangerKeywords.some((keyword) => sql.toLowerCase().includes(keyword.toLowerCase()))) { + throw new Error('SQL statements contain dangerous keywords'); + } +};