diff --git a/routes/search.ts b/routes/search.ts index 07d0fcddaee..41b5a3e42b8 100644 --- a/routes/search.ts +++ b/routes/search.ts @@ -20,7 +20,7 @@ export function searchProducts () { return (req: Request, res: Response, next: NextFunction) => { let criteria: any = req.query.q === 'undefined' ? '' : req.query.q ?? '' criteria = (criteria.length <= 200) ? criteria : criteria.substring(0, 200) - models.sequelize.query(`SELECT * FROM Products WHERE ((name LIKE '%${criteria}%' OR description LIKE '%${criteria}%') AND deletedAt IS NULL) ORDER BY name`) // vuln-code-snippet vuln-line unionSqlInjectionChallenge dbSchemaChallenge + models.sequelize.query('SELECT * FROM Products WHERE ((name LIKE :criteria OR description LIKE :criteria) AND deletedAt IS NULL) ORDER BY name', { replacements: { criteria: `%${criteria}%` } }) // vuln-code-snippet vuln-line unionSqlInjectionChallenge dbSchemaChallenge .then(([products]: any) => { const dataString = JSON.stringify(products) if (challengeUtils.notSolved(challenges.unionSqlInjectionChallenge)) { // vuln-code-snippet hide-start diff --git a/rsn/cache.json b/rsn/cache.json index 731f4648d47..5daa7361dd4 100644 --- a/rsn/cache.json +++ b/rsn/cache.json @@ -689,7 +689,8 @@ 2, 6, 7, - 8 + 8, + 9 ] }, "directoryListingChallenge_1_correct.ts": { @@ -1267,7 +1268,9 @@ }, "unionSqlInjectionChallenge_1.ts": { "added": [], - "removed": [] + "removed": [ + 6 + ] }, "unionSqlInjectionChallenge_2_correct.ts": { "added": [ @@ -1285,7 +1288,8 @@ 6, 7, 8, - 9 + 9, + 10 ] }, "weakPasswordChallenge_1_correct.ts": { diff --git a/test/api/search.test.ts b/test/api/search.test.ts index 828748eda09..06d52c7301b 100644 --- a/test/api/search.test.ts +++ b/test/api/search.test.ts @@ -39,141 +39,65 @@ void describe('/rest/products/search', () => { assert.equal(res.body.data.length, 1) }) - void it('GET product search fails with error message that exposes ins SQL Injection vulnerability', async () => { + void it('GET product search does not fail with an SQL error on a quote in the criteria', async () => { const res = await request(app) .get("/rest/products/search?q=';") - assert.equal(res.status, 500) - assert.ok(res.headers['content-type']?.includes('text/html')) - assert.ok(res.text.includes(`

${config.get('application.name')} (Express`)) - assert.ok(res.text.includes('SQLITE_ERROR: near ";": syntax error')) - }) - - void it('GET product search SQL Injection fails from two missing closing parenthesis', async () => { - const res = await request(app) - .get("/rest/products/search?q=' union select id,email,password from users--") - assert.equal(res.status, 500) - assert.ok(res.headers['content-type']?.includes('text/html')) - assert.ok(res.text.includes(`

${config.get('application.name')} (Express`)) - assert.ok(res.text.includes('SQLITE_ERROR: near "union": syntax error')) - }) - - void it('GET product search SQL Injection fails from one missing closing parenthesis', async () => { - const res = await request(app) - .get("/rest/products/search?q=') union select id,email,password from users--") - assert.equal(res.status, 500) - assert.ok(res.headers['content-type']?.includes('text/html')) - assert.ok(res.text.includes(`

${config.get('application.name')} (Express`)) - assert.ok(res.text.includes('SQLITE_ERROR: near "union": syntax error')) - }) - - void it('GET product search SQL Injection fails for SELECT * FROM attack due to wrong number of returned columns', async () => { - const res = await request(app) - .get("/rest/products/search?q=')) union select * from users--") - assert.equal(res.status, 500) - assert.ok(res.headers['content-type']?.includes('text/html')) - assert.ok(res.text.includes(`

${config.get('application.name')} (Express`)) - assert.ok(res.text.includes('SQLITE_ERROR: SELECTs to the left and right of UNION do not have the same number of result columns')) - }) - - void it('GET product search can create UNION SELECT with Users table and fixed columns', async () => { - const res = await request(app) - .get("/rest/products/search?q=')) union select '1','2','3','4','5','6','7','8','9' from users--") assert.equal(res.status, 200) assert.ok(res.headers['content-type']?.includes('application/json')) - const match = res.body.data.find((item: any) => - item.id === '1' && item.name === '2' && item.description === '3' && - item.price === '4' && item.deluxePrice === '5' && item.image === '6' && - item.createdAt === '7' && item.updatedAt === '8' - ) - assert.ok(match, 'Expected to find a row with fixed column values from UNION SELECT') + assert.equal(res.body.data.length, 0) }) - void it('GET product search can create UNION SELECT with Users table and required columns', async () => { + void it('GET product search treats a UNION SELECT payload as a literal search term', async () => { const res = await request(app) .get("/rest/products/search?q=')) union select id,'2','3',email,password,'6','7','8','9' from users--") assert.equal(res.status, 200) assert.ok(res.headers['content-type']?.includes('application/json')) - - const adminMatch = res.body.data.find((item: any) => - item.id === 1 && item.price === `admin@${config.get('application.domain')}` && item.deluxePrice === security.hash('admin123') - ) - assert.ok(adminMatch, 'Expected admin user in UNION SELECT results') - - const jimMatch = res.body.data.find((item: any) => - item.id === 2 && item.price === `jim@${config.get('application.domain')}` && item.deluxePrice === security.hash('ncc-1701') - ) - assert.ok(jimMatch, 'Expected jim user in UNION SELECT results') - - const benderMatch = res.body.data.find((item: any) => - item.id === 3 && item.price === `bender@${config.get('application.domain')}` - ) - assert.ok(benderMatch, 'Expected bender user in UNION SELECT results') - - const bjoernMatch = res.body.data.find((item: any) => - item.id === 4 && item.price === 'bjoern.kimminich@gmail.com' && item.deluxePrice === security.hash('bW9jLmxpYW1nQGhjaW5pbW1pay5ucmVvamI=') - ) - assert.ok(bjoernMatch, 'Expected bjoern user in UNION SELECT results') - - const cisoMatch = res.body.data.find((item: any) => - item.id === 5 && item.price === `ciso@${config.get('application.domain')}` && item.deluxePrice === security.hash('mDLx?94T~1CfVfZMzw@sJ9f?s3L6lbMqE70FfI8^54jbNikY5fymx7c!YbJb') - ) - assert.ok(cisoMatch, 'Expected ciso user in UNION SELECT results') - - const supportMatch = res.body.data.find((item: any) => - item.id === 6 && item.price === `support@${config.get('application.domain')}` && item.deluxePrice === security.hash('J6aVjTgOpRs@?5l!Zkq2AYnCE@RF$P') - ) - assert.ok(supportMatch, 'Expected support user in UNION SELECT results') + assert.equal(res.body.data.length, 0) }) - void it('GET product search can create UNION SELECT with sqlite_master table and required column', async () => { + void it('GET product search cannot exfiltrate the users table via UNION SELECT', async () => { const res = await request(app) - .get("/rest/products/search?q=')) union select sql,'2','3','4','5','6','7','8','9' from sqlite_master--") + .get("/rest/products/search?q=')) union select '1','2','3','4','5','6','7','8','9' from users--") assert.equal(res.status, 200) assert.ok(res.headers['content-type']?.includes('application/json')) + assert.equal(res.body.data.length, 0) - const basketItemsMatch = res.body.data.find((item: any) => - item.id === 'CREATE TABLE `BasketItems` (`ProductId` INTEGER REFERENCES `Products` (`id`) ON DELETE CASCADE ON UPDATE CASCADE, `BasketId` INTEGER REFERENCES `Baskets` (`id`) ON DELETE CASCADE ON UPDATE CASCADE, `id` INTEGER PRIMARY KEY AUTOINCREMENT, `quantity` INTEGER, `createdAt` DATETIME NOT NULL, `updatedAt` DATETIME NOT NULL, UNIQUE (`ProductId`, `BasketId`))' - ) - assert.ok(basketItemsMatch, 'Expected BasketItems CREATE TABLE in UNION SELECT results') - - const sqliteSequenceMatch = res.body.data.find((item: any) => - item.id === 'CREATE TABLE sqlite_sequence(name,seq)' - ) - assert.ok(sqliteSequenceMatch, 'Expected sqlite_sequence CREATE TABLE in UNION SELECT results') + const dataString = JSON.stringify(res.body.data) + assert.ok(!dataString.includes(`admin@${config.get('application.domain')}`)) + assert.ok(!dataString.includes(security.hash('admin123'))) }) - void it('GET product search cannot select logically deleted christmas special by default', async () => { + void it('GET product search cannot exfiltrate the database schema via UNION SELECT', async () => { const res = await request(app) - .get('/rest/products/search?q=seasonal%20special%20offer') + .get("/rest/products/search?q=')) union select sql,'2','3','4','5','6','7','8','9' from sqlite_master--") assert.equal(res.status, 200) assert.ok(res.headers['content-type']?.includes('application/json')) assert.equal(res.body.data.length, 0) + assert.ok(!JSON.stringify(res.body.data).includes('CREATE TABLE')) }) - void it('GET product search by description cannot select logically deleted christmas special due to forced early where-clause termination', async () => { + void it('GET product search cannot select logically deleted christmas special by default', async () => { const res = await request(app) - .get("/rest/products/search?q=seasonal%20special%20offer'))--") + .get('/rest/products/search?q=seasonal%20special%20offer') assert.equal(res.status, 200) assert.ok(res.headers['content-type']?.includes('application/json')) assert.equal(res.body.data.length, 0) }) - void it('GET product search can select logically deleted christmas special by forcibly commenting out the remainder of where clause', async () => { + void it('GET product search cannot select logically deleted christmas special by commenting out the remainder of the where clause', async () => { const res = await request(app) .get(`/rest/products/search?q=${christmasProduct.name}'))--`) assert.equal(res.status, 200) assert.ok(res.headers['content-type']?.includes('application/json')) - assert.equal(res.body.data.length, 1) - assert.equal(res.body.data[0].name, christmasProduct.name) + assert.equal(res.body.data.length, 0) }) - void it('GET product search can select logically deleted unsafe product by forcibly commenting out the remainder of where clause', async () => { + void it('GET product search cannot select logically deleted unsafe product by commenting out the remainder of the where clause', async () => { const res = await request(app) .get(`/rest/products/search?q=${pastebinLeakProduct.name}'))--`) assert.equal(res.status, 200) assert.ok(res.headers['content-type']?.includes('application/json')) - assert.equal(res.body.data.length, 1) - assert.equal(res.body.data[0].name, pastebinLeakProduct.name) + assert.equal(res.body.data.length, 0) }) void it('GET product search with empty search parameter returns all products', async () => { diff --git a/test/cypress/e2e/search.spec.ts b/test/cypress/e2e/search.spec.ts index a97fefa5655..cb22c198c0b 100644 --- a/test/cypress/e2e/search.spec.ts +++ b/test/cypress/e2e/search.spec.ts @@ -31,49 +31,36 @@ describe('/#/search', () => { }) describe('/rest/products/search', () => { - describe('challenge "unionSqlInjection"', () => { - it('query param in product search endpoint should be susceptible to UNION SQL injection attacks', () => { - cy.request( - "/rest/products/search?q=')) union select id,'2','3',email,password,'6','7','8','9' from users--" - ) - cy.expectChallengeSolved({ challenge: 'User Credentials' }) - }) - }) - - describe('challenge "dbSchema"', () => { - it('query param in product search endpoint should be susceptible to UNION SQL injection attacks', () => { - cy.request( - "/rest/products/search?q=')) union select sql,'2','3','4','5','6','7','8','9' from sqlite_master--" - ) - cy.expectChallengeSolved({ challenge: 'Database Schema' }) - }) + it('query param in product search endpoint should not be susceptible to UNION SQL injection attacks', () => { + cy.request( + "/rest/products/search?q=')) union select id,'2','3',email,password,'6','7','8','9' from users--" + ) + .its('body') + .then((body) => { + expect(body.data).to.have.length(0) + }) }) - describe('challenge "dlpPastebinLeakChallenge"', () => { - beforeEach(() => { - cy.login({ - email: 'admin', - password: 'admin123' + it('query param in product search endpoint should not expose the database schema', () => { + cy.request( + "/rest/products/search?q=')) union select sql,'2','3','4','5','6','7','8','9' from sqlite_master--" + ) + .its('body') + .then((body) => { + expect(body.data).to.have.length(0) }) - }) - - it('search query should logically reveal the special product', () => { - cy.request("/rest/products/search?q='))--") - .its('body') - .then((sourceContent) => { - cy.task('GetPastebinLeakProduct').then((pastebinLeakProduct: Product) => { - let foundProduct = false + }) - sourceContent.data.forEach((product: Product) => { - if (product.name === pastebinLeakProduct.name) { - foundProduct = true - } - }) - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - expect(foundProduct).to.be.true - }) + it('search query should not reveal logically deleted products', () => { + cy.request("/rest/products/search?q='))--") + .its('body') + .then((sourceContent) => { + cy.task('GetPastebinLeakProduct').then((pastebinLeakProduct: Product) => { + const foundProduct = sourceContent.data.some((product: Product) => product.name === pastebinLeakProduct.name) + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + expect(foundProduct).to.be.false }) - }) + }) }) xdescribe('challenge "christmasSpecial"', () => {