From 738b5af4c0912b02cf76063391501b51464c884a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 3 Jun 2021 17:45:19 +0100 Subject: [PATCH] Switching from .toString to .toSQL().toNative() for sql injection protection. --- packages/server/src/integrations/base/sql.js | 24 ++++--- .../server/src/integrations/tests/sql.spec.js | 63 +++++++++++++------ 2 files changed, 60 insertions(+), 27 deletions(-) diff --git a/packages/server/src/integrations/base/sql.js b/packages/server/src/integrations/base/sql.js index 11d803cedc..a0fe9bd0eb 100644 --- a/packages/server/src/integrations/base/sql.js +++ b/packages/server/src/integrations/base/sql.js @@ -47,7 +47,7 @@ function addFilters(query, filters) { function buildCreate(knex, json) { const { endpoint, body } = json let query = knex(endpoint.entityId) - return query.insert(body).toString() + return query.insert(body) } function buildRead(knex, json, limit) { @@ -78,21 +78,21 @@ function buildRead(knex, json, limit) { } else { query.limit(limit) } - return query.toString() + return query } function buildUpdate(knex, json) { const { endpoint, body, filters } = json let query = knex(endpoint.entityId) query = addFilters(query, filters) - return query.update(body).toString() + return query.update(body) } function buildDelete(knex, json) { const { endpoint, filters } = json let query = knex(endpoint.entityId) query = addFilters(query, filters) - return query.delete().toString() + return query.delete() } class SqlQueryBuilder { @@ -102,22 +102,28 @@ class SqlQueryBuilder { this._limit = limit } - query(json) { + _query(json) { const { endpoint } = json const knex = require("knex")({ client: this._client }) const operation = endpoint.operation + let query switch (operation) { case Operation.CREATE: - return buildCreate(knex, json) + query = buildCreate(knex, json) + break case Operation.READ: - return buildRead(knex, json, this._limit) + query = buildRead(knex, json, this._limit) + break case Operation.UPDATE: - return buildUpdate(knex, json) + query = buildUpdate(knex, json) + break case Operation.DELETE: - return buildDelete(knex, json) + query = buildDelete(knex, json) + break default: throw `Operation ${operation} type is not supported by SQL query builder` } + return query.toSQL().toNative() } } diff --git a/packages/server/src/integrations/tests/sql.spec.js b/packages/server/src/integrations/tests/sql.spec.js index 58e6356bda..8bfa6f765c 100644 --- a/packages/server/src/integrations/tests/sql.spec.js +++ b/packages/server/src/integrations/tests/sql.spec.js @@ -54,30 +54,39 @@ describe("SQL query builder", () => { }) it("should test a basic read", () => { - const query = sql.query(generateReadJson()) - expect(query).toEqual(`select * from "${TABLE_NAME}" limit ${limit}`) + const query = sql._query(generateReadJson()) + expect(query).toEqual({ + bindings: [limit], + sql: `select * from "${TABLE_NAME}" limit $1` + }) }) it("should test a read with specific columns", () => { - const query = sql.query(generateReadJson({ + const query = sql._query(generateReadJson({ fields: ["name", "age"] })) - expect(query).toEqual(`select "name", "age" from "${TABLE_NAME}" limit ${limit}`) + expect(query).toEqual({ + bindings: [limit], + sql: `select "name", "age" from "${TABLE_NAME}" limit $1` + }) }) it("should test a where string starts with read", () => { - const query = sql.query(generateReadJson({ + const query = sql._query(generateReadJson({ filters: { string: { name: "John", } } })) - expect(query).toEqual(`select * from "${TABLE_NAME}" where "name" like 'John%' limit ${limit}`) + expect(query).toEqual({ + bindings: ["John%", limit], + sql: `select * from "${TABLE_NAME}" where "name" like $1 limit $2` + }) }) it("should test a where range read", () => { - const query = sql.query(generateReadJson({ + const query = sql._query(generateReadJson({ filters: { range: { age: { @@ -87,44 +96,62 @@ describe("SQL query builder", () => { } } })) - expect(query).toEqual(`select * from "${TABLE_NAME}" where "age" between 2 and 10 limit ${limit}`) + expect(query).toEqual({ + bindings: [2, 10, limit], + sql: `select * from "${TABLE_NAME}" where "age" between $1 and $2 limit $3` + }) }) it("should test an create statement", () => { - const query = sql.query(generateCreateJson(TABLE_NAME, { + const query = sql._query(generateCreateJson(TABLE_NAME, { name: "Michael", age: 45, })) - expect(query).toEqual(`insert into "${TABLE_NAME}" ("age", "name") values (45, 'Michael')`) + expect(query).toEqual({ + bindings: [45, "Michael"], + sql: `insert into "${TABLE_NAME}" ("age", "name") values ($1, $2)` + }) }) it("should test an update statement", () => { - const query = sql.query(generateUpdateJson(TABLE_NAME, { + const query = sql._query(generateUpdateJson(TABLE_NAME, { name: "John" }, { equal: { id: 1001, } })) - expect(query).toEqual(`update "${TABLE_NAME}" set "name" = 'John' where "id" = 1001`) + expect(query).toEqual({ + bindings: ["John", 1001], + sql: `update "${TABLE_NAME}" set "name" = $1 where "id" = $2` + }) }) it("should test a delete statement", () => { - const query = sql.query(generateDeleteJson(TABLE_NAME, { + const query = sql._query(generateDeleteJson(TABLE_NAME, { equal: { id: 1001, } })) - expect(query).toEqual(`delete from "${TABLE_NAME}" where "id" = 1001`) + expect(query).toEqual({ + bindings: [1001], + sql: `delete from "${TABLE_NAME}" where "id" = $1` + }) }) it("should work with MS-SQL", () => { - const query = new Sql("mssql", 10).query(generateReadJson()) - expect(query).toEqual(`select top (10) * from [${TABLE_NAME}]`) + const query = new Sql("mssql", 10)._query(generateReadJson()) + expect(query).toEqual({ + bindings: [10], + sql: `select top (@p0) * from [${TABLE_NAME}]` + }) }) it("should work with mySQL", () => { - const query = new Sql("mysql", 10).query(generateReadJson()) - expect(query).toEqual(`select * from \`${TABLE_NAME}\` limit 10`) + const query = new Sql("mysql", 10)._query(generateReadJson()) + expect(query).toEqual({ + bindings: [10], + sql: `select * from \`${TABLE_NAME}\` limit ?` + }) }) })