Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate query before execute #3048

Merged
merged 3 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/runtime/api/query.post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ import { eventHandler, getRouterParam, readValidatedBody } from 'h3'
import * as z from 'zod'
import type { RuntimeConfig } from '@nuxt/content'
import loadDatabaseAdapter, { checkAndImportDatabaseIntegrity } from '../internal/database.server'
import { assertSafeQuery } from '../internal/security'
import { useRuntimeConfig } from '#imports'

export default eventHandler(async (event) => {
const { sql } = await readValidatedBody(event, z.object({ sql: z.string() }).parse)
const collection = getRouterParam(event, 'collection')!

assertSafeQuery(sql, collection)

const conf = useRuntimeConfig().content as RuntimeConfig['content']
await checkAndImportDatabaseIntegrity(event, collection, conf)

Expand Down
68 changes: 68 additions & 0 deletions src/runtime/internal/security.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
const SQL_COMMANDS = /SELECT|INSERT|UPDATE|DELETE|DROP|ALTER/i

/**
* Assert that the query is safe
* A query will consider safe if it matched the output pattern of query builder
* Any mismatch will be considered as unsafe
*
* @param sql - The SQL query to check
* @param collection - The collection to check
* @returns True if the query is safe, false otherwise
*/
export function assertSafeQuery(sql: string, collection: string) {
const match = sql.match(/^SELECT (.*) FROM (\w+)( WHERE .*)? ORDER BY (["\w,\s]+) (ASC|DESC)( LIMIT \d+)?( OFFSET \d+)?$/)
if (!match) {
throw new Error('Invalid query')
}

const [_, select, from, where, orderBy, order, limit, offset] = match

// COLUMNS
const columns = select.trim().split(', ')
if (columns.length === 1) {
if (
columns[0] !== '*'
&& !columns[0].startsWith('COUNT(')
&& !columns[0].match(/^COUNT\((DISTINCT )?[a-z_]\w+\) as count$/)
) {
throw new Error('Invalid query')
}
}
else if (!columns.every(column => column.match(/^"[a-z_]\w+"$/i))) {
throw new Error('Invalid query')
}

// FROM
if (from !== `_content_${collection}`) {
throw new Error('Invalid query')
}

// WHERE
if (where) {
if (!where.startsWith(' WHERE (') || !where.endsWith(')')) {
throw new Error('Invalid query')
}
const noString = where?.replace(/(['"`])(?:\\.|[^\\])*?\1/g, '')
if (noString.match(SQL_COMMANDS)) {
throw new Error('Invalid query')
}
}

// ORDER BY
const _order = (orderBy + ' ' + order).split(', ')
if (!_order.every(column => column.match(/^("[a-z_]+"|[a-z_]+) (ASC|DESC)$/))) {
throw new Error('Invalid query')
}

// LIMIT
if (limit !== undefined && !limit.match(/^ LIMIT \d+$/)) {
throw new Error('Invalid query')
}

// OFFSET
if (offset !== undefined && !offset.match(/^ OFFSET \d+$/)) {
throw new Error('Invalid query')
}

return true
}
98 changes: 98 additions & 0 deletions test/unit/assertSafeQuery.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { assertSafeQuery } from '../../src/runtime/internal/security'
import { collectionQueryBuilder } from '../../src/runtime/internal/query'

// Mock tables from manifest
vi.mock('#content/manifest', () => ({
tables: {
test: '_content_test',
},
}))
const mockFetch = vi.fn().mockResolvedValue(Promise.resolve([{}]))
const mockCollection = 'test' as never

describe('decompressSQLDump', () => {
beforeEach(() => {
mockFetch.mockClear()
})

const queries = {
'SELECT * FROM sqlite_master': false,
'INSERT INTO _test VALUES (\'abc\')': false,
'CREATE TABLE _test (id TEXT PRIMARY KEY)': false,
'select * from _content_test ORDER BY id DESC': false,
' SELECT * FROM _content_test ORDER BY id DESC': false,
'SELECT * FROM _content_test ORDER BY id DESC ': false,
'SELECT * FROM _content_test ORDER BY id DESC': true,
'SELECT * FROM _content_test ORDER BY id ASC,stem DESC': false,
'SELECT * FROM _content_test ORDER BY id ASC, stem DESC': true,
'SELECT * FROM _content_test ORDER BY id DESC -- comment is not allowed': false,
'SELECT * FROM _content_test ORDER BY id DESC; SELECT * FROM _content_test ORDER BY id DESC': false,
'SELECT * FROM _content_test ORDER BY id DESC LIMIT 10': true,
'SELECT * FROM _content_test ORDER BY id DESC LIMIT 10 OFFSET 10': true,
// Where clause should follow query builder syntax
'SELECT * FROM _content_test WHERE id = 1 ORDER BY id DESC LIMIT 10 OFFSET 10': false,
'SELECT * FROM _content_test WHERE (id = 1) ORDER BY id DESC LIMIT 10 OFFSET 10': true,
'SELECT * FROM _content_test WHERE (id = \'");\'); select * from ((SELECT * FROM sqlite_master where 1 <> "") as t) ORDER BY type DESC': false,
}

Object.entries(queries).forEach(([query, isValid]) => {
it(`${query}`, () => {
if (isValid) {
expect(() => assertSafeQuery(query, 'test')).not.toThrow()
}
else {
expect(() => assertSafeQuery(query, 'test')).toThrow()
}
})
})

it('throws error if query is not valid', async () => {
await collectionQueryBuilder(mockCollection, mockFetch).all()
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()

await collectionQueryBuilder(mockCollection, mockFetch).count('stem')
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()

await collectionQueryBuilder(mockCollection, mockFetch).count('stem', true)
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()

await collectionQueryBuilder(mockCollection, mockFetch).first()
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()

await collectionQueryBuilder(mockCollection, mockFetch).order('stem', 'DESC').first()
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()

await collectionQueryBuilder(mockCollection, mockFetch).order('stem', 'DESC').order('id', 'ASC').first()
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()

await collectionQueryBuilder(mockCollection, mockFetch)
.select('stem', 'id', 'title')
.order('stem', 'DESC').order('id', 'ASC').first()
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()

await collectionQueryBuilder(mockCollection, mockFetch)
.select('stem', 'id', 'title')
.limit(10)
.andWhere(group => group.where('id', '=', 1).where('stem', '=', 'abc'))
.order('stem', 'DESC').order('id', 'ASC').first()
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()

await collectionQueryBuilder(mockCollection, mockFetch)
.select('stem', 'id', 'title')
.limit(10)
.andWhere(group => group.where('id', '=', 1).where('stem', '=', 'abc'))
.orWhere(group => group.where('id', '=', 2).where('stem', '=', 'def'))
.order('stem', 'DESC').order('id', 'ASC').first()
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()

await collectionQueryBuilder(mockCollection, mockFetch)
.select('stem', 'id', 'title')
.limit(10)
.andWhere(group => group.where('id', '=', 1).where('stem', '=', 'abc'))
.orWhere(group => group.where('id', '=', 2).where('stem', '=', 'def'))
.andWhere(group => group.where('id', '=', 3).orWhere(g => g.where('stem', '=', 'ghi')))
.order('stem', 'DESC').order('id', 'ASC').first()
expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()
})
})
Loading