From b925e8ccf0895b33b9f7281d4fee0633410573f2 Mon Sep 17 00:00:00 2001 From: Dimitris Halatsis Date: Mon, 28 Oct 2019 16:42:13 +0200 Subject: [PATCH] backport #1782 to 3.x (#1785) --- lib/utils.js | 35 +++++++++++++++++++++++++++++++++++ test/basic-querying.test.js | 15 ++++++++++++++- test/memory.test.js | 15 ++++++++++++++- test/util.test.js | 22 ++++++++++++++++++++++ 4 files changed, 85 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 3b10e0e3..c8226f9d 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -26,6 +26,7 @@ exports.findIndexOf = findIndexOf; exports.collectTargetIds = collectTargetIds; exports.idName = idName; exports.rankArrayElements = rankArrayElements; +exports.escapeRegExp = escapeRegExp; const g = require('strong-globalize')(); const traverse = require('traverse'); @@ -315,6 +316,36 @@ function isProhibited(key, prohibitedKeys) { return false; } +/** + * Accept an operator key and return whether it is used for a regular expression query or not + * @param {string} operator + * @returns {boolean} + */ +function isRegExpOperator(operator) { + return ['like', 'nlike', 'ilike', 'nilike', 'regexp'].includes(operator); +} + +/** + * Accept a RegExp string and make sure that any special characters for RegExp are escaped in case they + * create an invalid Regexp + * @param {string} str + * @returns {string} + */ +function escapeRegExp(str) { + assert.strictEqual(typeof str, 'string', 'String required for regexp escaping'); + try { + new RegExp(str); // try to parse string as regexp + return str; + } catch (unused) { + console.warn( + 'Auto-escaping invalid RegExp value %j supplied by the caller. ' + + 'Please note this behavior may change in the future.', + str + ); + return str.replace(/[\-\[\]\/\{\}\(\)\+\?\.\\\^\$\|]/g, '\\$&'); + } +} + /** * Sanitize the query object * @param query {object} The query object @@ -390,6 +421,10 @@ function sanitizeQuery(query, options) { return x; } + if (isRegExpOperator(this.key) && typeof x === 'string') { // we have regexp supporting operator and string to escape + return escapeRegExp(x); + } + return x; }); diff --git a/test/basic-querying.test.js b/test/basic-querying.test.js index cf53f73d..2e5a79c4 100644 --- a/test/basic-querying.test.js +++ b/test/basic-querying.test.js @@ -23,6 +23,7 @@ describe('basic-querying', function() { birthday: {type: Date, index: true}, role: {type: String, index: true}, order: {type: Number, index: true, sort: true}, + tag: {type: String, index: true}, vip: {type: Boolean}, address: { street: String, @@ -591,6 +592,12 @@ describe('basic-querying', function() { }); }); + it('should sanitize invalid usage of like', async () => { + const users = await User.find({where: {tag: {like: '['}}}); + users.should.have.length(1); + users[0].should.have.property('name', 'John Lennon'); + }); + it('should support "like" that is not satisfied', function(done) { User.find({where: {name: {like: 'Bob'}}}, @@ -616,6 +623,11 @@ describe('basic-querying', function() { done(); }); }); + + it('should properly sanitize invalid ilike filter', async () => { + const users = await User.find({where: {name: {ilike: '['}}}); + users.should.be.empty(); + }); }); const itWhenNilikeSupported = connectorCapabilities.nilike !== false; @@ -820,7 +832,7 @@ describe('basic-querying', function() { sample({name: true}).expect(['name']); sample({name: false}).expect([ - 'id', 'seq', 'email', 'role', 'order', 'birthday', 'vip', 'address', 'friends', 'addressLoc', + 'id', 'seq', 'email', 'role', 'order', 'birthday', 'vip', 'address', 'friends', 'addressLoc', 'tag', ]); sample({name: false, id: true}).expect(['id']); sample({id: true}).expect(['id']); @@ -1363,6 +1375,7 @@ function seed(done) { birthday: new Date('1980-12-08'), order: 2, vip: true, + tag: '[singer]', address: { street: '123 A St', city: 'San Jose', diff --git a/test/memory.test.js b/test/memory.test.js index 80b1bc44..42404fab 100644 --- a/test/memory.test.js +++ b/test/memory.test.js @@ -201,6 +201,7 @@ describe('Memory connector', function() { birthday: {type: Date, index: true}, role: {type: String, index: true}, order: {type: Number, index: true, sort: true}, + tag: {type: String, index: true}, vip: {type: Boolean}, address: { street: String, @@ -229,6 +230,12 @@ describe('Memory connector', function() { }); }); + it('should properly sanitize like invalid query', async () => { + const users = await User.find({where: {tag: {like: '['}}}); + users.should.have.length(1); + users[0].should.have.property('name', 'John Lennon'); + }); + it('should allow to find using like with regexp', function(done) { User.find({where: {name: {like: /.*St.*/}}}, function(err, posts) { should.not.exist(err); @@ -253,6 +260,11 @@ describe('Memory connector', function() { }); }); + it('should sanitize nlike invalid query', async () => { + const users = await User.find({where: {name: {nlike: '['}}}); + users.should.have.length(6); + }); + it('should allow to find using nlike with regexp', function(done) { User.find({where: {name: {nlike: /.*St.*/}}}, function(err, posts) { should.not.exist(err); @@ -513,7 +525,7 @@ describe('Memory connector', function() { }); it('should support the regexp operator with regex strings', function(done) { - User.find({where: {name: {regexp: '^J'}}}, function(err, users) { + User.find({where: {name: {regexp: 'non$'}}}, function(err, users) { should.not.exist(err); users.length.should.equal(1); users[0].name.should.equal('John Lennon'); @@ -562,6 +574,7 @@ describe('Memory connector', function() { role: 'lead', birthday: new Date('1980-12-08'), vip: true, + tag: '[singer]', address: { street: '123 A St', city: 'San Jose', diff --git a/test/util.test.js b/test/util.test.js index 312c90de..a814ec85 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -115,6 +115,28 @@ describe('util.sanitizeQuery', function() { sanitizeQuery(q2, {prohibitedKeys: ['secret']}); q2.should.eql({and: [{}, {x: 1}]}); }); + + it('should allow proper structured regexp string', () => { + const q1 = {where: {name: {like: '^J'}}}; + sanitizeQuery(q1).should.eql({where: {name: {like: '^J'}}}); + }); + + it('should properly sanitize regexp string operators', () => { + const q1 = {where: {name: {like: '['}}}; + sanitizeQuery(q1).should.eql({where: {name: {like: '\\['}}}); + + const q2 = {where: {name: {nlike: '['}}}; + sanitizeQuery(q2).should.eql({where: {name: {nlike: '\\['}}}); + + const q3 = {where: {name: {ilike: '['}}}; + sanitizeQuery(q3).should.eql({where: {name: {ilike: '\\['}}}); + + const q4 = {where: {name: {nilike: '['}}}; + sanitizeQuery(q4).should.eql({where: {name: {nilike: '\\['}}}); + + const q5 = {where: {name: {regexp: '['}}}; + sanitizeQuery(q5).should.eql({where: {name: {regexp: '\\['}}}); + }); }); describe('util.parseSettings', function() {