Skip to content
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
28 changes: 28 additions & 0 deletions .github/workflows/security.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,31 @@ jobs:
with:
image-ref: app:${{ github.sha }}
severity: CRITICAL,HIGH

dast-scan:
name: DAST Scan (SQLMap)
runs-on: ubuntu-latest
if: github.event_name == 'workflow_dispatch'
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.x'

- name: Install SQLMap
run: pip install sqlmap

- name: Start Application Container
run: |
docker compose -f docker-compose.staging.yml up -d
# Wait for application to be healthy
sleep 15

- name: Run SQLMap Scan
run: |
# Scanning course search endpoint dynamically with SQLMap (non-blocking)
sqlmap -u "http://localhost:3000/search?q=test" --batch --crawl=2 --level=1 --risk=1
continue-on-error: true
100 changes: 100 additions & 0 deletions docs/security/sql-injection-prevention.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# SQL Injection Prevention & Query Best Practices Guide

This document outlines the architectural standards and implementation guidelines to prevent SQL Injection (SQLi) vulnerabilities in the TeachLink backend.

---

## 1. TypeORM Query Builder Best Practices

When building queries using TypeORM's `QueryBuilder`, **never** concatenate or interpolate variables directly into the query string. Always use parameter placeholders and bind the values using the parameters object.

### ❌ Vulnerable (Direct Interpolation)
```typescript
// DANGEROUS: Allows SQL Injection if query is malicious
const courses = await courseRepository
.createQueryBuilder('course')
.where(`course.title ILIKE '%${query}%'`)
.getMany();
```

### ✅ Secure (Parameterized Query)
```typescript
// SAFE: Parameterized placeholders (:query)
const courses = await courseRepository
.createQueryBuilder('course')
.where('course.title ILIKE :query', { query: `%${query}%` })
.getMany();
```

---

## 2. Parameterizing Raw SQL Queries

When execution of raw SQL queries is necessary (e.g. using `dataSource.query` or `manager.query`), you must always use parameter placeholders (`$1`, `$2` for PostgreSQL) and pass parameters in an array.

### ❌ Vulnerable (Direct Interpolation)
```typescript
// DANGEROUS: Executes raw input strings
const result = await dataSource.query(
`SELECT * FROM courses WHERE price >= ${minPrice}`
);
```

### ✅ Secure (Parameterized Array)
```typescript
// SAFE: Uses postgres placeholders and passes values separately
const result = await dataSource.query(
'SELECT * FROM courses WHERE price >= $1',
[minPrice]
);
```

---

## 3. Dynamic SQL Identifiers (Table & Column Names)

In rare cases where identifiers like table names, column names, or savepoint names must be dynamic (since SQL parameters cannot bind to identifiers), you must run strict validation against a whitelist regex:

### Whitelist Regex Validation
* **Pattern**: `/^[a-zA-Z_][a-zA-Z0-9_]*$/`
* **Rule**: Throw a validation error immediately if the identifier does not match this pattern.

### ✅ Secure Identifier Validation Pattern
```typescript
function validateSqlIdentifier(name: string): void {
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(name)) {
throw new BadRequestException(`Invalid database identifier: ${name}`);
}
}

// Inside migration or transaction helper:
validateSqlIdentifier(tableName);
await manager.query(`SELECT * FROM "${tableName}"`);
```

---

## 4. Escaping Wildcard Characters in LIKE/ILIKE

When querying using `LIKE` or `ILIKE` operators, users can pass special wildcard characters (`%` or `_`) to alter the query logic. To prevent wildcard abuse, use the `sanitizeSqlLike` utility:

```typescript
import { sanitizeSqlLike } from '../common/utils/sanitization.utils';

const safeQuery = sanitizeSqlLike(query);

await tenantRepository
.createQueryBuilder('tenant')
.where("tenant.name ILIKE :query ESCAPE '\\'", { query: `%${safeQuery}%` })
.getMany();
```

---

## 5. Security Checklist for Code Reviews

* [ ] No template strings or `+` concatenations are present inside `where()`, `andWhere()`, or `orWhere()` clauses.
* [ ] No raw queries are executed using interpolated templates (e.g., `dataSource.query("SELECT ... ${var}")`).
* [ ] Any custom table names, column names, or transaction savepoint names are validated using the identifier regex pattern `/^[a-zA-Z_][a-zA-Z0-9_]*$/`.
* [ ] Transaction timeouts are validated as positive integer numbers before execution.
* [ ] User searches matching standard text are sanitized using `sanitizeSqlLike` and utilize the `ESCAPE '\\'` operator.
12 changes: 12 additions & 0 deletions src/common/database/transaction-helper.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,20 +86,29 @@ export class TransactionHelperService {
* Create savepoint for nested transactions
*/
async createSavepoint(manager: EntityManager, savepointName: string): Promise<void> {
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(savepointName)) {
throw new Error(`Invalid savepoint name: ${savepointName}`);
}
await manager.query(`SAVEPOINT ${savepointName}`);
this.logger.debug(`Created savepoint: ${savepointName}`);
}
/**
* Rollback to savepoint
*/
async rollbackToSavepoint(manager: EntityManager, savepointName: string): Promise<void> {
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(savepointName)) {
throw new Error(`Invalid savepoint name: ${savepointName}`);
}
await manager.query(`ROLLBACK TO SAVEPOINT ${savepointName}`);
this.logger.debug(`Rolled back to savepoint: ${savepointName}`);
}
/**
* Release savepoint
*/
async releaseSavepoint(manager: EntityManager, savepointName: string): Promise<void> {
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(savepointName)) {
throw new Error(`Invalid savepoint name: ${savepointName}`);
}
await manager.query(`RELEASE SAVEPOINT ${savepointName}`);
this.logger.debug(`Released savepoint: ${savepointName}`);
}
Expand All @@ -124,6 +133,9 @@ export class TransactionHelperService {
* Set transaction timeout
*/
async setTransactionTimeout(manager: EntityManager, timeoutMs: number): Promise<void> {
if (!Number.isInteger(timeoutMs) || timeoutMs < 0) {
throw new Error(`Invalid timeout value: ${timeoutMs}`);
}
await manager.query(`SET LOCK_TIMEOUT ${timeoutMs}`);
}
}
14 changes: 14 additions & 0 deletions src/sharding/migration/shard-migration.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,17 @@ export class ShardMigrationService {
): Promise<void> {
if (rows.length === 0) return;

if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(table)) {
throw new BadRequestException(`Invalid table name: ${table}`);
}

const columns = Object.keys(rows[0]);
for (const col of columns) {
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(col)) {
throw new BadRequestException(`Invalid column name: ${col}`);
}
}

const values: unknown[] = [];
const rowPlaceholders: string[] = [];

Expand Down Expand Up @@ -233,5 +243,9 @@ export class ShardMigrationService {
if (plan.batchSize <= 0 || plan.batchSize > 10_000) {
throw new BadRequestException('batchSize must be between 1 and 10,000');
}

if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(plan.entityType)) {
throw new BadRequestException(`Invalid table name: ${plan.entityType}`);
}
}
}
7 changes: 5 additions & 2 deletions test/jest-sanity.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
{
"moduleFileExtensions": ["ts", "js", "json"],
"rootDir": ".",
"testEnvironment": "./test/utils/test-environment.js",
"testEnvironment": "./utils/test-environment.js",
"testRegex": "test/sanity/.*\\.ts$",
"transform": {
"^.+\\.(t|j)s$": "ts-jest"
},
"setupFilesAfterEnv": ["<rootDir>/test/setup.ts"],
"setupFilesAfterEnv": ["<rootDir>/setup.ts"],
"moduleNameMapper": {
"^uuid$": "<rootDir>/mocks/uuid.ts"
},
"testTimeout": 15000,
"forceExit": true,
"verbose": true,
Expand Down
Loading
Loading