From 62ebfe9a92360f5e61d03714f8e36f0fb2680c90 Mon Sep 17 00:00:00 2001 From: Mohammad AbuAboud Date: Thu, 28 Mar 2024 00:35:11 +0000 Subject: [PATCH] fix(mysql/postgres): add markdown warning how to prevent sql injections --- package-lock.json | 22 ++++++++++++++++--- package.json | 4 +++- packages/pieces/community/mysql/package.json | 2 +- .../mysql/src/lib/actions/delete-row.ts | 22 ++++++++++--------- .../mysql/src/lib/actions/execute-query.ts | 5 +++-- .../mysql/src/lib/actions/find-rows.ts | 17 ++++++-------- .../mysql/src/lib/actions/insert-row.ts | 19 +++++----------- .../mysql/src/lib/actions/update-row.ts | 15 ++++--------- .../community/mysql/src/lib/common/index.ts | 22 +++++++++++-------- .../pieces/community/postgres/package.json | 2 +- .../postgres/src/lib/actions/run-query.ts | 5 +++++ 11 files changed, 74 insertions(+), 61 deletions(-) diff --git a/package-lock.json b/package-lock.json index edbce83e44..05cadb20c3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -161,6 +161,7 @@ "soap": "1.0.0", "socket.io": "4.7.2", "sqlite3": "5.1.7", + "sqlstring": "2.3.3", "ssh2-sftp-client": "9.1.0", "string-replace-async": "3.0.2", "stripe": "14.3.0", @@ -230,6 +231,7 @@ "@types/quill": "2.0.10", "@types/semver": "7.5.6", "@types/snowflake-sdk": "1.6.20", + "@types/sqlstring": "2.3.2", "@types/ssh2-sftp-client": "9.0.0", "@types/tinycolor2": "1.4.5", "@types/xml2js": "0.4.14", @@ -12856,6 +12858,12 @@ "@types/node": "*" } }, + "node_modules/@types/sqlstring": { + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/@types/sqlstring/-/sqlstring-2.3.2.tgz", + "integrity": "sha512-lVRe4Iz9UNgiHelKVo8QlC8fb5nfY8+p+jNQNE+UVsuuVlQnWhyWmQ/wF5pE8Ys6TdjfVpqTG9O9i2vi6E0+Sg==", + "dev": true + }, "node_modules/@types/ssh2": { "version": "1.11.19", "resolved": "https://registry.npmjs.org/@types/ssh2/-/ssh2-1.11.19.tgz", @@ -28020,6 +28028,14 @@ "util-deprecate": "~1.0.1" } }, + "node_modules/mysql/node_modules/sqlstring": { + "version": "2.3.1", + "resolved": "https://registry.npmjs.org/sqlstring/-/sqlstring-2.3.1.tgz", + "integrity": "sha512-ooAzh/7dxIG5+uDik1z/Rd1vli0+38izZhGzSa34FwR7IbelPWCCKSNIl8jlL/F7ERvy8CB2jNeM1E9i9mXMAQ==", + "engines": { + "node": ">= 0.6" + } + }, "node_modules/mz": { "version": "2.7.0", "resolved": "https://registry.npmjs.org/mz/-/mz-2.7.0.tgz", @@ -34860,9 +34876,9 @@ "optional": true }, "node_modules/sqlstring": { - "version": "2.3.1", - "resolved": "https://registry.npmjs.org/sqlstring/-/sqlstring-2.3.1.tgz", - "integrity": "sha512-ooAzh/7dxIG5+uDik1z/Rd1vli0+38izZhGzSa34FwR7IbelPWCCKSNIl8jlL/F7ERvy8CB2jNeM1E9i9mXMAQ==", + "version": "2.3.3", + "resolved": "https://registry.npmjs.org/sqlstring/-/sqlstring-2.3.3.tgz", + "integrity": "sha512-qC9iz2FlN7DQl3+wjwn3802RTyjCx7sDvfQEXchwa6CWOx07/WVfh91gBmQ9fahw8snwGEWU3xGzOt4tFyHLxg==", "engines": { "node": ">= 0.6" } diff --git a/package.json b/package.json index 2513e7bde5..a1eeedba6a 100644 --- a/package.json +++ b/package.json @@ -174,6 +174,7 @@ "soap": "1.0.0", "socket.io": "4.7.2", "sqlite3": "5.1.7", + "sqlstring": "2.3.3", "ssh2-sftp-client": "9.1.0", "string-replace-async": "3.0.2", "stripe": "14.3.0", @@ -243,6 +244,7 @@ "@types/quill": "2.0.10", "@types/semver": "7.5.6", "@types/snowflake-sdk": "1.6.20", + "@types/sqlstring": "2.3.2", "@types/ssh2-sftp-client": "9.0.0", "@types/tinycolor2": "1.4.5", "@types/xml2js": "0.4.14", @@ -296,4 +298,4 @@ "nx": { "includedScripts": [] } -} \ No newline at end of file +} diff --git a/packages/pieces/community/mysql/package.json b/packages/pieces/community/mysql/package.json index 81a6005368..64769478a8 100644 --- a/packages/pieces/community/mysql/package.json +++ b/packages/pieces/community/mysql/package.json @@ -1,4 +1,4 @@ { "name": "@activepieces/piece-mysql", - "version": "0.1.4" + "version": "0.1.5" } \ No newline at end of file diff --git a/packages/pieces/community/mysql/src/lib/actions/delete-row.ts b/packages/pieces/community/mysql/src/lib/actions/delete-row.ts index 2879c1a78b..882e0b4952 100644 --- a/packages/pieces/community/mysql/src/lib/actions/delete-row.ts +++ b/packages/pieces/community/mysql/src/lib/actions/delete-row.ts @@ -1,6 +1,7 @@ import { createAction, Property } from '@activepieces/pieces-framework'; -import { mysqlCommon, mysqlConnect, sanitizeColumnName } from '../common'; +import { mysqlCommon, mysqlConnect, sanitizeColumnName, warningMarkdown } from '../common'; import { mysqlAuth } from '../..'; +import sqlstring from 'sqlstring'; export default createAction({ auth: mysqlAuth, @@ -8,6 +9,7 @@ export default createAction({ displayName: 'Delete Row', description: 'Deletes one or more rows from a table', props: { + markdown: warningMarkdown, timezone: mysqlCommon.timezone, table: mysqlCommon.table(), search_column: Property.ShortText({ @@ -20,18 +22,18 @@ export default createAction({ }), }, async run(context) { - const qs = - 'DELETE FROM `' + - context.propsValue.table + - '` WHERE ' + - sanitizeColumnName(context.propsValue.search_column) + - '=?;'; - const conn = await mysqlConnect(context.auth, context.propsValue); + const tableName = sanitizeColumnName(context.propsValue.table); + const searchColumn = sanitizeColumnName(context.propsValue.search_column); + const searchValue = context.propsValue.search_value; + + const queryString = `DELETE FROM \`${tableName}\` WHERE ${searchColumn}=?;`; + + const connection = await mysqlConnect(context.auth, context.propsValue); try { - const result = await conn.query(qs, [context.propsValue.search_value]); + const result = await connection.query(queryString, [searchValue]); return result; } finally { - await conn.end(); + await connection.end(); } }, }); diff --git a/packages/pieces/community/mysql/src/lib/actions/execute-query.ts b/packages/pieces/community/mysql/src/lib/actions/execute-query.ts index 70c09d9588..d2bab1739e 100644 --- a/packages/pieces/community/mysql/src/lib/actions/execute-query.ts +++ b/packages/pieces/community/mysql/src/lib/actions/execute-query.ts @@ -1,5 +1,5 @@ import { createAction, Property } from '@activepieces/pieces-framework'; -import { mysqlCommon, mysqlConnect } from '../common'; +import { mysqlCommon, mysqlConnect, warningMarkdown } from '../common'; import { mysqlAuth } from '../..'; export default createAction({ @@ -8,6 +8,7 @@ export default createAction({ displayName: 'Execute Query', description: 'Executes a query on the mysql database and returns the results', props: { + markdown: warningMarkdown, timezone: mysqlCommon.timezone, query: Property.ShortText({ displayName: 'Query', @@ -16,7 +17,7 @@ export default createAction({ }), args: Property.Array({ displayName: 'Arguments', - description: 'Can inserted in the query string using ?', + description: 'Arguments to use in the query, if any. Should be in the same order as the ? in the query string..', required: false, }), }, diff --git a/packages/pieces/community/mysql/src/lib/actions/find-rows.ts b/packages/pieces/community/mysql/src/lib/actions/find-rows.ts index 231bebfc3e..d7e529ff5e 100644 --- a/packages/pieces/community/mysql/src/lib/actions/find-rows.ts +++ b/packages/pieces/community/mysql/src/lib/actions/find-rows.ts @@ -1,5 +1,5 @@ import { createAction, Property } from '@activepieces/pieces-framework'; -import { mysqlCommon, mysqlConnect, isSpecialColumn } from '../common'; +import { mysqlCommon, mysqlConnect, sanitizeColumnName, warningMarkdown } from '../common'; import { mysqlAuth } from '../..'; export default createAction({ @@ -8,6 +8,7 @@ export default createAction({ displayName: 'Find Rows', description: 'Reads rows from a table', props: { + markdown: warningMarkdown, timezone: mysqlCommon.timezone, table: mysqlCommon.table(), condition: Property.ShortText({ @@ -29,17 +30,13 @@ export default createAction({ async run(context) { const columns = (context.propsValue.columns as string[]) || ['*']; const qsColumns = columns - .map((c) => (isSpecialColumn(c) ? c : '`' + c + '`')) + .map((c) => sanitizeColumnName(c)) .join(','); - const qs = - 'SELECT ' + - qsColumns + - ' FROM `' + - context.propsValue.table + - '` WHERE ' + - context.propsValue.condition + - ';'; + + const qs = `SELECT ${qsColumns} FROM \`${sanitizeColumnName(context.propsValue.table)}\` WHERE ${context.propsValue.condition};`; + const conn = await mysqlConnect(context.auth, context.propsValue); + try { const results = await conn.query(qs, context.propsValue.args); return { results }; diff --git a/packages/pieces/community/mysql/src/lib/actions/insert-row.ts b/packages/pieces/community/mysql/src/lib/actions/insert-row.ts index 04cea5a5d7..5b16d3f368 100644 --- a/packages/pieces/community/mysql/src/lib/actions/insert-row.ts +++ b/packages/pieces/community/mysql/src/lib/actions/insert-row.ts @@ -1,6 +1,7 @@ import { createAction, Property } from '@activepieces/pieces-framework'; -import { mysqlCommon, mysqlConnect } from '../common'; +import { mysqlCommon, mysqlConnect, sanitizeColumnName, warningMarkdown } from '../common'; import { mysqlAuth } from '../..'; +import sqlstring from 'sqlstring'; export default createAction({ auth: mysqlAuth, @@ -12,25 +13,17 @@ export default createAction({ table: mysqlCommon.table(), values: Property.Object({ displayName: 'Values', - description: 'Values to be inserted into the row', required: true, }), }, async run(context) { const fields = Object.keys(context.propsValue.values); - const values = fields.map((f) => context.propsValue.values[f]); - const qsFields = fields.map((f) => '`' + f + '`').join(','); - const qsValues = fields.map((f) => '?').join(','); - const qs = - 'INSERT INTO `' + - context.propsValue.table + - '` (' + - qsFields + - ') VALUES (' + - qsValues + - ');'; + const qsFields = fields.map((f) => sanitizeColumnName(f)).join(','); + const qsValues = fields.map(() => '?').join(','); + const qs = `INSERT INTO \`${sanitizeColumnName(context.propsValue.table)}\` (${qsFields}) VALUES (${qsValues});`; const conn = await mysqlConnect(context.auth, context.propsValue); try { + const values = fields.map((f) => context.propsValue.values[f]); const result = await conn.query(qs, values); return result; } finally { diff --git a/packages/pieces/community/mysql/src/lib/actions/update-row.ts b/packages/pieces/community/mysql/src/lib/actions/update-row.ts index 709cea5a17..665d5114cf 100644 --- a/packages/pieces/community/mysql/src/lib/actions/update-row.ts +++ b/packages/pieces/community/mysql/src/lib/actions/update-row.ts @@ -1,6 +1,7 @@ import { createAction, Property } from '@activepieces/pieces-framework'; import { mysqlCommon, mysqlConnect, sanitizeColumnName } from '../common'; import { mysqlAuth } from '../..'; +import sqlstring from 'sqlstring'; export default createAction({ auth: mysqlAuth, @@ -12,7 +13,6 @@ export default createAction({ table: mysqlCommon.table(), values: Property.Object({ displayName: 'Values', - description: 'Values to be updated', required: true, }), search_column: Property.ShortText({ @@ -26,18 +26,11 @@ export default createAction({ }, async run(context) { const fields = Object.keys(context.propsValue.values); - const values = fields.map((f) => context.propsValue.values[f]); - const qsValues = fields.map((f) => '`' + f + '`=?').join(','); - const qs = - 'UPDATE `' + - context.propsValue.table + - '` SET ' + - qsValues + - ' WHERE ' + - sanitizeColumnName(context.propsValue.search_column) + - '=?;'; + const qsValues = fields.map((f) => sanitizeColumnName(f) + '=?').join(','); + const qs = `UPDATE \`${sanitizeColumnName(context.propsValue.table)}\` SET ${qsValues} WHERE ${sqlstring.escape(context.propsValue.search_column)}=?;`; const conn = await mysqlConnect(context.auth, context.propsValue); try { + const values = fields.map((f) => context.propsValue.values[f]); const result = await conn.query(qs, [ ...values, context.propsValue.search_value, diff --git a/packages/pieces/community/mysql/src/lib/common/index.ts b/packages/pieces/community/mysql/src/lib/common/index.ts index 2fefc1461e..f2aa197453 100644 --- a/packages/pieces/community/mysql/src/lib/common/index.ts +++ b/packages/pieces/community/mysql/src/lib/common/index.ts @@ -5,6 +5,14 @@ import { } from '@activepieces/pieces-framework'; import { Connection, createConnection } from 'promise-mysql'; import { mysqlAuth } from '../..'; +import sqlstring from 'sqlstring'; + +export const warningMarkdown = Property.MarkDown({ + value: ` + **DO NOT** use dynamic input directly in the query string or column names. + \n + Use **?** in the query and dynamic values in args/values for parameterized queries to prevent **SQL injection**.` +}); export async function mysqlConnect( auth: PiecePropValueSchema, @@ -29,12 +37,11 @@ export async function mysqlGetTableNames(conn: Connection): Promise { export const mysqlCommon = { timezone: Property.ShortText({ displayName: 'Timezone', - description: 'Timezone for the mysql server to use', + description: 'Timezone for the MySQL server to use', required: false, }), table: (required = true) => Property.Dropdown({ - description: 'The name of the table', displayName: 'Table', required, refreshers: [], @@ -42,7 +49,7 @@ export const mysqlCommon = { if (!auth) { return { disabled: true, - placeholder: 'connect to your database first', + placeholder: 'Connect to your database first', options: [], }; } @@ -65,13 +72,10 @@ export const mysqlCommon = { }), }; -export function isSpecialColumn(name: string): boolean { - return name == '*' || name.includes(' ') || name.includes('('); -} -export function sanitizeColumnName(name: string): string { - if (isSpecialColumn(name)) { +export function sanitizeColumnName(name: string | undefined): string { + if ( name == '*') { return name; } - return '`' + name + '`'; + return sqlstring.escapeId(name); } diff --git a/packages/pieces/community/postgres/package.json b/packages/pieces/community/postgres/package.json index 9c8f6b406f..9a41beb41f 100644 --- a/packages/pieces/community/postgres/package.json +++ b/packages/pieces/community/postgres/package.json @@ -1,4 +1,4 @@ { "name": "@activepieces/piece-postgres", - "version": "0.1.4" + "version": "0.1.5" } \ No newline at end of file diff --git a/packages/pieces/community/postgres/src/lib/actions/run-query.ts b/packages/pieces/community/postgres/src/lib/actions/run-query.ts index 7592dae7b2..91887b733b 100644 --- a/packages/pieces/community/postgres/src/lib/actions/run-query.ts +++ b/packages/pieces/community/postgres/src/lib/actions/run-query.ts @@ -8,6 +8,11 @@ export const runQuery = createAction({ displayName: 'Run Query', description: 'Run Query', props: { + markdown: Property.MarkDown({ + value: ` + **DO NOT** insert dynamic input directly into the query string. Instead, use $1, $2, $3 and add them in args for parameterized queries to prevent **SQL injection.**` + }), + query: Property.ShortText({ displayName: 'Query', description: 'Please use $1, $2, etc. for parameterized queries to avoid SQL injection.',