From a940d14256e4f516b1ad6547fbf1bd5a4aa17b2d Mon Sep 17 00:00:00 2001 From: joan Date: Mon, 22 Nov 2021 14:40:03 +0100 Subject: [PATCH 1/3] fix(filterValue): fixed sql injection vulnerability Refs: 3369 --- front/core/components/field/style.scss | 2 +- .../entry/front/latest-buys-search-panel/index.html | 13 ++++++++----- .../entry/front/latest-buys-search-panel/index.js | 12 ------------ modules/item/back/methods/tag/filterValue.js | 11 ++++++++--- modules/item/front/search-panel/index.html | 12 +++++++----- modules/item/front/search-panel/index.js | 12 ------------ 6 files changed, 24 insertions(+), 38 deletions(-) diff --git a/front/core/components/field/style.scss b/front/core/components/field/style.scss index ceaeda40a..9012b8c4c 100644 --- a/front/core/components/field/style.scss +++ b/front/core/components/field/style.scss @@ -71,7 +71,7 @@ color: $color-font; &::placeholder { - color: $color-font-bg; + color: $color-font-bg-marginal; } &[type=time], &[type=date], diff --git a/modules/entry/front/latest-buys-search-panel/index.html b/modules/entry/front/latest-buys-search-panel/index.html index 4693141f8..c4f9a4a72 100644 --- a/modules/entry/front/latest-buys-search-panel/index.html +++ b/modules/entry/front/latest-buys-search-panel/index.html @@ -81,18 +81,21 @@ on-change="itemTag.value = null"> - + show-field="value" + value-field="value" + rule> { const where = filter.where; if (where && where.value) { stmt.merge(conn.makeWhere({value: {like: `%${where.value}%`}})); - stmt.merge(` - ORDER BY value LIKE '${where.value}' DESC, - value LIKE '${where.value}%' DESC`); + + const newStmt = new ParameterizedSQL( + `ORDER BY value LIKE ? DESC, + value LIKE ? DESC`, [ + where.value, + `${where.value}%` + ]); + ParameterizedSQL.append(newStmt, stmt); } stmt.merge(conn.makeLimit(filter)); diff --git a/modules/item/front/search-panel/index.html b/modules/item/front/search-panel/index.html index 57f05bb54..42e68c71c 100644 --- a/modules/item/front/search-panel/index.html +++ b/modules/item/front/search-panel/index.html @@ -72,19 +72,21 @@ - + show-field="value" + value-field="value" + rule> Date: Mon, 22 Nov 2021 15:49:32 +0100 Subject: [PATCH 2/3] Updated unit tests --- .../front/latest-buys-search-panel/index.js | 2 +- .../latest-buys-search-panel/index.spec.js | 53 ++++++++++++------- modules/item/front/search-panel/index.js | 2 +- modules/item/front/search-panel/index.spec.js | 53 ++++++++++++------- 4 files changed, 70 insertions(+), 40 deletions(-) diff --git a/modules/entry/front/latest-buys-search-panel/index.js b/modules/entry/front/latest-buys-search-panel/index.js index ce9eddcca..83dc88724 100644 --- a/modules/entry/front/latest-buys-search-panel/index.js +++ b/modules/entry/front/latest-buys-search-panel/index.js @@ -57,7 +57,7 @@ class Controller extends SearchPanel { removeField(index, field) { this.fieldFilters.splice(index, 1); - this.$.filter[field] = undefined; + delete this.$.filter[field]; } } diff --git a/modules/entry/front/latest-buys-search-panel/index.spec.js b/modules/entry/front/latest-buys-search-panel/index.spec.js index 6d403b9fb..9e187a25a 100644 --- a/modules/entry/front/latest-buys-search-panel/index.spec.js +++ b/modules/entry/front/latest-buys-search-panel/index.spec.js @@ -12,33 +12,48 @@ describe('Entry', () => { controller = $componentController('vnLatestBuysSearchPanel', {$element}); })); - describe('getSourceTable()', () => { - it(`should return null if there's no selection`, () => { - let selection = null; - let result = controller.getSourceTable(selection); + describe('filter() setter', () => { + it(`should set the tags property to the scope filter with an empty array`, () => { + const expectedFilter = { + tags: [{}] + }; + controller.filter = null; - expect(result).toBeNull(); + expect(controller.filter).toEqual(expectedFilter); }); - it(`should return null if there's a selection but its isFree property is truthy`, () => { - let selection = {isFree: true}; - let result = controller.getSourceTable(selection); + it(`should set the tags property to the scope filter with an array of tags`, () => { + const expectedFilter = { + description: 'My item', + tags: [{}] + }; + const expectedFieldFilter = [{ + info: { + label: 'description', + name: 'description', + type: null + }, + name: 'description', + value: 'My item' + }]; + controller.filter = { + description: 'My item' + }; - expect(result).toBeNull(); + expect(controller.filter).toEqual(expectedFilter); + expect(controller.fieldFilters).toEqual(expectedFieldFilter); }); + }); - it(`should return the formated sourceTable concatenated to a path`, () => { - let selection = {sourceTable: 'hello guy'}; - let result = controller.getSourceTable(selection); + describe('removeField()', () => { + it(`should remove the description property from the fieldFilters and from the scope filter`, () => { + const expectedFilter = {tags: [{}]}; + controller.filter = {description: 'My item'}; - expect(result).toEqual('Hello guys'); - }); + controller.removeField(0, 'description'); - it(`should return a path if there's no sourceTable and the selection has an id`, () => { - let selection = {id: 99}; - let result = controller.getSourceTable(selection); - - expect(result).toEqual(`ItemTags/filterItemTags/${selection.id}`); + expect(controller.filter).toEqual(expectedFilter); + expect(controller.fieldFilters).toEqual([]); }); }); }); diff --git a/modules/item/front/search-panel/index.js b/modules/item/front/search-panel/index.js index 6a0f358eb..2448728be 100644 --- a/modules/item/front/search-panel/index.js +++ b/modules/item/front/search-panel/index.js @@ -57,7 +57,7 @@ class Controller extends SearchPanel { removeField(index, field) { this.fieldFilters.splice(index, 1); - this.$.filter[field] = undefined; + delete this.$.filter[field]; } } diff --git a/modules/item/front/search-panel/index.spec.js b/modules/item/front/search-panel/index.spec.js index 0e2b7f848..39b5b7aa5 100644 --- a/modules/item/front/search-panel/index.spec.js +++ b/modules/item/front/search-panel/index.spec.js @@ -12,33 +12,48 @@ describe('Item', () => { controller = $componentController('vnItemSearchPanel', {$element}); })); - describe('getSourceTable()', () => { - it(`should return null if there's no selection`, () => { - let selection = null; - let result = controller.getSourceTable(selection); + describe('filter() setter', () => { + it(`should set the tags property to the scope filter with an empty array`, () => { + const expectedFilter = { + tags: [{}] + }; + controller.filter = null; - expect(result).toBeNull(); + expect(controller.filter).toEqual(expectedFilter); }); - it(`should return null if there's a selection but its isFree property is truthy`, () => { - let selection = {isFree: true}; - let result = controller.getSourceTable(selection); + it(`should set the tags property to the scope filter with an array of tags`, () => { + const expectedFilter = { + description: 'My item', + tags: [{}] + }; + const expectedFieldFilter = [{ + info: { + label: 'description', + name: 'description', + type: null + }, + name: 'description', + value: 'My item' + }]; + controller.filter = { + description: 'My item' + }; - expect(result).toBeNull(); + expect(controller.filter).toEqual(expectedFilter); + expect(controller.fieldFilters).toEqual(expectedFieldFilter); }); + }); - it(`should return the formated sourceTable concatenated to a path`, () => { - let selection = {sourceTable: 'hello guy'}; - let result = controller.getSourceTable(selection); + describe('removeField()', () => { + it(`should remove the description property from the fieldFilters and from the scope filter`, () => { + const expectedFilter = {tags: [{}]}; + controller.filter = {description: 'My item'}; - expect(result).toEqual('Hello guys'); - }); + controller.removeField(0, 'description'); - it(`should return a path if there's no sourceTable and the selection has an id`, () => { - let selection = {id: 99}; - let result = controller.getSourceTable(selection); - - expect(result).toEqual(`ItemTags/filterItemTags/${selection.id}`); + expect(controller.filter).toEqual(expectedFilter); + expect(controller.fieldFilters).toEqual([]); }); }); }); From 4ebad6b372d989e579d67d62cd338e3eac5aa415 Mon Sep 17 00:00:00 2001 From: joan Date: Mon, 22 Nov 2021 15:54:01 +0100 Subject: [PATCH 3/3] filter order was not properly being applied --- modules/item/back/methods/tag/filterValue.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/item/back/methods/tag/filterValue.js b/modules/item/back/methods/tag/filterValue.js index 6344a5299..3f39519fc 100644 --- a/modules/item/back/methods/tag/filterValue.js +++ b/modules/item/back/methods/tag/filterValue.js @@ -48,13 +48,13 @@ module.exports = Self => { if (where && where.value) { stmt.merge(conn.makeWhere({value: {like: `%${where.value}%`}})); - const newStmt = new ParameterizedSQL( + const orderStmt = new ParameterizedSQL( `ORDER BY value LIKE ? DESC, value LIKE ? DESC`, [ where.value, `${where.value}%` ]); - ParameterizedSQL.append(newStmt, stmt); + ParameterizedSQL.append(stmt, orderStmt); } stmt.merge(conn.makeLimit(filter));