Skip to content

Fix SQL injection via orderBy direction in Drizzle adapter#22

Merged
gblikas merged 4 commits intomainfrom
copilot/fix-sql-injection-drizzle-adapter
Mar 16, 2026
Merged

Fix SQL injection via orderBy direction in Drizzle adapter#22
gblikas merged 4 commits intomainfrom
copilot/fix-sql-injection-drizzle-adapter

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 16, 2026

The orderBy direction parameter was passed to sql.raw() without validation when the field wasn't found in the schema, enabling SQL injection in ORDER BY context.

Changes

  • Security fix (src/adapters/drizzle/index.ts): Normalize direction to ASC/DESC only before passing to sql.raw()
  • Security tests (src/adapters/drizzle/drizzle-adapter.test.ts): Added 7 tests covering DROP TABLE, SELECT, DELETE injection vectors

Before/After

// Before (vulnerable)
orderClauses.push(
  sql`${sql.identifier(field)} ${sql.raw(direction.toUpperCase())}`
);

// After (safe)
const safeDirection = direction.toLowerCase() === 'asc' ? 'ASC' : 'DESC';
orderClauses.push(
  sql`${sql.identifier(field)} ${sql.raw(safeDirection)}`
);

Malicious input like 'asc; DROP TABLE todos;--' now normalizes to 'ASC'.

Original prompt

This section details on the original issue you should resolve

<issue_title>Security: SQL injection via orderBy direction in Drizzle adapter</issue_title>
<issue_description>## Summary

The Drizzle adapter passes the orderBy direction parameter to sql.raw() without validation when the field is not found in the schema. Since QueryKit is often used in contexts where query parameters originate from client input (e.g., Next.js server actions, REST APIs), this creates a SQL injection vector.

Affected Code

src/adapters/drizzle/index.ts lines 166-180:

Object.entries(options.orderBy).forEach(([field, direction]) => {
  const schemaField = this.getSchemaField(tableName, field);

  if (schemaField) {
    // Safe: uses Drizzle typed functions
    orderClauses.push(
      direction === 'asc' ? asc(schemaField) : desc(schemaField)
    );
  } else {
    // Vulnerable: direction passed to sql.raw() without validation
    orderClauses.push(
      sql`${sql.identifier(field)} ${sql.raw(direction.toUpperCase())}`
    );
  }
});

Attack Vector

When orderBy is called with:

  1. A field name that does not exist in the schema (triggering the else branch)
  2. A malicious direction value

The direction is passed directly to sql.raw(), enabling SQL injection in the ORDER BY context.

// Attacker-controlled input
qk.query('users')
  .where('status:active')
  .orderBy('nonExistentField', 'asc; SELECT password FROM users--')
  .execute();

Impact

  • Blind SQL injection in ORDER BY context
  • While WHERE-clause filters remain intact, ORDER BY subqueries can read from any table
  • Enables cross-tenant data exfiltration via timing/error-based oracles
  • Severity increases when QueryKit is exposed to client-controlled parameters

Current Safe Path

When the field is in the schema, the code is safe:

direction === 'asc' ? asc(schemaField) : desc(schemaField)

This boolean comparison means malicious direction values simply result in desc() being used—no injection possible.

Suggested Fix

Apply the same safe pattern to the fallback path:

} else {
  // Validate direction strictly before using sql.raw()
  const safeDirection = direction === 'asc' ? 'ASC' : 'DESC';
  orderClauses.push(
    sql`${sql.identifier(field)} ${sql.raw(safeDirection)}`
  );
}

This ensures that regardless of input, only ASC or DESC reaches the SQL.

Alternative Approaches

  1. Throw on invalid direction: Reject any direction that isn't strictly 'asc' or 'desc'
  2. Add security config option: Introduce allowedOrderFields in the security configuration to restrict which fields can be used for ordering
  3. Warn/reject unknown fields: When a field isn't in the schema, either warn or reject rather than falling back to raw SQL

Suggested Unit Tests

describe('DrizzleAdapter orderBy security', () => {
  it('should safely handle direction when field exists in schema', async () => {
    const result = await adapter.execute('users', expression, {
      orderBy: { name: 'asc' }
    });
    // Verify query executed successfully
  });

  it('should safely handle direction when field is NOT in schema', async () => {
    // This should NOT allow SQL injection
    const result = await adapter.execute('users', expression, {
      orderBy: { unknownField: 'asc; DROP TABLE users;--' }
    });
    // Verify only ASC or DESC appears in generated SQL
  });

  it('should normalize direction to ASC/DESC only', async () => {
    const maliciousDirections = [
      'asc; SELECT * FROM secrets',
      'desc); DROP TABLE users;--',
      'ASC LIMIT 1; --',
      '" OR 1=1 --',
    ];
    
    for (const direction of maliciousDirections) {
      // Should either:
      // - Normalize to ASC/DESC
      // - Throw an error
      // - NOT execute raw SQL with the malicious input
    }
  });

  it('should handle empty/null direction safely', async () => {
    const result = await adapter.execute('users', expression, {
      orderBy: { name: '' as any }
    });
    // Should default to a safe direction or throw
  });
});

Environment

  • QueryKit version: 0.6.1
  • Drizzle ORM version: 0.38.x
  • Database: PostgreSQL (likely affects all SQL databases)

Workaround

Applications can mitigate this by validating orderBy parameters before passing to QueryKit:

const VALID_DIRECTIONS = new Set(['asc', 'desc']);

function safeOrderBy(field: string, direction: string) {
  const normalizedDirection = direction.toLowerCase();
  if (!VALID_DIRECTIONS.has(normalizedDirection)) {
    throw new Error('Invalid orderBy direction');
  }
  return qk.query('table').where(query).orderBy(field, normalizedDirection);
}

However, this defense should ideally be built into QueryKit itself.
</issue_description>

<agent_instructions>Validate that this issue exists. If it does, fix it by creating unit tests first, testing (examining failure). A...


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
querykit Ready Ready Preview, Comment Mar 16, 2026 7:16pm

Co-authored-by: gblikas <13577108+gblikas@users.noreply.github.com>
- Validate direction parameter strictly before using sql.raw()
- Normalize any input to either 'ASC' or 'DESC' only
- Add comprehensive security tests for SQL injection prevention
- Tests verify malicious payloads (DROP TABLE, SELECT, DELETE, etc.) are blocked

Co-authored-by: gblikas <13577108+gblikas@users.noreply.github.com>
Copilot AI changed the title [WIP] [SEC-456] Fix SQL injection via orderBy direction in Drizzle adapter Fix SQL injection via orderBy direction in Drizzle adapter Mar 16, 2026
Copilot AI requested a review from gblikas March 16, 2026 19:09
@gblikas gblikas marked this pull request as ready for review March 16, 2026 19:11
Changed injection payloads to reference the actual 'todos' table defined
in the test schema instead of a non-existent 'users' table. This makes
the tests more meaningful by demonstrating threats against the real
data structure.

Co-authored-by: gblikas <13577108+gblikas@users.noreply.github.com>
@gblikas gblikas merged commit 4b4d07b into main Mar 16, 2026
5 checks passed
@gblikas gblikas deleted the copilot/fix-sql-injection-drizzle-adapter branch March 16, 2026 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: SQL injection via orderBy direction in Drizzle adapter

2 participants