Skip to content

Commit 5f780b6

Browse files
chamikaJchamikaJ
authored
Merge pull request #368 from Worklenz/fix/sql-injection-vulnerabilities
fix: Address SQL injection vulnerabilities by introducing secure SQL …
2 parents 374835a + 6f99f00 commit 5f780b6

3 files changed

Lines changed: 63 additions & 24 deletions

File tree

‎manage.sh‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,6 +1108,11 @@ push_images() {
11081108
# Push frontend images
11091109
print_info "Pushing frontend images to Docker Hub..."
11101110
if [ "$frontend_version" != "latest" ]; then
1111+
# Verify version-tagged image exists locally
1112+
if ! docker image inspect "${docker_username}/worklenz-frontend:${frontend_version}" >/dev/null 2>&1; then
1113+
print_error "Image ${docker_username}/worklenz-frontend:${frontend_version} not found locally. Please build it first."
1114+
return 1
1115+
fi
11111116
print_info "Pushing ${docker_username}/worklenz-frontend:${frontend_version}..."
11121117
docker push "${docker_username}/worklenz-frontend:${frontend_version}"
11131118
if [ $? -ne 0 ]; then
@@ -1317,16 +1322,32 @@ install_worklenz() {
13171322
if [ "$backend_version" != "latest" ]; then
13181323
print_info "Pushing backend image (${backend_version})..."
13191324
docker push "${docker_username}/worklenz-backend:${backend_version}"
1325+
if [ $? -ne 0 ]; then
1326+
print_error "Failed to push backend image (${backend_version})"
1327+
return 1
1328+
fi
13201329
fi
13211330
print_info "Pushing backend image (latest)..."
13221331
docker push "${docker_username}/worklenz-backend:latest"
1332+
if [ $? -ne 0 ]; then
1333+
print_error "Failed to push backend image (latest)"
1334+
return 1
1335+
fi
13231336

13241337
if [ "$frontend_version" != "latest" ]; then
13251338
print_info "Pushing frontend image (${frontend_version})..."
13261339
docker push "${docker_username}/worklenz-frontend:${frontend_version}"
1340+
if [ $? -ne 0 ]; then
1341+
print_error "Failed to push frontend image (${frontend_version})"
1342+
return 1
1343+
fi
13271344
fi
13281345
print_info "Pushing frontend image (latest)..."
13291346
docker push "${docker_username}/worklenz-frontend:latest"
1347+
if [ $? -ne 0 ]; then
1348+
print_error "Failed to push frontend image (latest)"
1349+
return 1
1350+
fi
13301351

13311352
print_success "Images pushed to Docker Hub!"
13321353
fi

‎worklenz-backend/src/controllers/reporting/projects/reporting-projects-controller.ts‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,26 +27,26 @@ export default class ReportingProjectsController extends ReportingProjectsBase {
2727
let statusesClause = "";
2828
if (req.query.statuses) {
2929
const statusIds = (req.query.statuses as string).split(",").filter(id => id.trim());
30-
const { clause, params } = SqlHelper.buildOptionalInClause(statusIds, 'p.status_id', paramOffset);
31-
statusesClause = clause;
30+
const { clause, params } = SqlHelper.buildOptionalInClause(statusIds, 'status_id', paramOffset);
31+
statusesClause = clause.replace('status_id', 'p.status_id');
3232
filterParams.push(...params);
3333
paramOffset += params.length;
3434
}
3535

3636
let healthsClause = "";
3737
if (req.query.healths) {
3838
const healthIds = (req.query.healths as string).split(",").filter(id => id.trim());
39-
const { clause, params } = SqlHelper.buildOptionalInClause(healthIds, 'p.health_id', paramOffset);
40-
healthsClause = clause;
39+
const { clause, params } = SqlHelper.buildOptionalInClause(healthIds, 'health_id', paramOffset);
40+
healthsClause = clause.replace('health_id', 'p.health_id');
4141
filterParams.push(...params);
4242
paramOffset += params.length;
4343
}
4444

4545
let categoriesClause = "";
4646
if (req.query.categories) {
4747
const categoryIds = (req.query.categories as string).split(",").filter(id => id.trim());
48-
const { clause, params } = SqlHelper.buildOptionalInClause(categoryIds, 'p.category_id', paramOffset);
49-
categoriesClause = clause;
48+
const { clause, params } = SqlHelper.buildOptionalInClause(categoryIds, 'category_id', paramOffset);
49+
categoriesClause = clause.replace('category_id', 'p.category_id');
5050
filterParams.push(...params);
5151
paramOffset += params.length;
5252
}
@@ -63,8 +63,8 @@ export default class ReportingProjectsController extends ReportingProjectsBase {
6363
let teamsClause = "";
6464
if (req.query.teams) {
6565
const teamIds = (req.query.teams as string).split(",").filter(id => id.trim());
66-
const { clause, params } = SqlHelper.buildOptionalInClause(teamIds, 'p.team_id', paramOffset);
67-
teamsClause = clause;
66+
const { clause, params } = SqlHelper.buildOptionalInClause(teamIds, 'team_id', paramOffset);
67+
teamsClause = clause.replace('team_id', 'p.team_id');
6868
filterParams.push(...params);
6969
paramOffset += params.length;
7070
}
@@ -149,7 +149,7 @@ export default class ReportingProjectsController extends ReportingProjectsBase {
149149
if (key === DATE_RANGES.LAST_QUARTER)
150150
return { clause: ",(SELECT (CURRENT_DATE - INTERVAL '3 months')::DATE) AS start_date, (SELECT (CURRENT_DATE)::DATE) AS end_date", params: [] };
151151
if (key === DATE_RANGES.ALL_TIME)
152-
return { clause: ",(SELECT (MIN(task_work_log.created_at)::DATE) FROM task_work_log WHERE task_id IN (SELECT id FROM tasks WHERE project_id = $1)) AS start_date, (SELECT (MAX(task_work_log.created_at)::DATE) FROM task_work_log WHERE task_id IN (SELECT id FROM tasks WHERE project_id = $1)) AS end_date", params: [] };
152+
return { clause: `,(SELECT (MIN(task_work_log.created_at)::DATE) FROM task_work_log WHERE task_id IN (SELECT id FROM tasks WHERE project_id = $${paramOffset})) AS start_date, (SELECT (MAX(task_work_log.created_at)::DATE) FROM task_work_log WHERE task_id IN (SELECT id FROM tasks WHERE project_id = $${paramOffset})) AS end_date`, params: [] };
153153

154154
return { clause: "", params: [] };
155155
}
@@ -272,26 +272,26 @@ export default class ReportingProjectsController extends ReportingProjectsBase {
272272
let statusesClause = "";
273273
if (req.query.statuses) {
274274
const statusIds = (req.query.statuses as string).split(",").filter(id => id.trim());
275-
const { clause, params } = SqlHelper.buildOptionalInClause(statusIds, 'p.status_id', paramOffset);
276-
statusesClause = clause;
275+
const { clause, params } = SqlHelper.buildOptionalInClause(statusIds, 'status_id', paramOffset);
276+
statusesClause = clause.replace('status_id', 'p.status_id');
277277
filterParams.push(...params);
278278
paramOffset += params.length;
279279
}
280280

281281
let healthsClause = "";
282282
if (req.query.healths) {
283283
const healthIds = (req.query.healths as string).split(",").filter(id => id.trim());
284-
const { clause, params } = SqlHelper.buildOptionalInClause(healthIds, 'p.health_id', paramOffset);
285-
healthsClause = clause;
284+
const { clause, params } = SqlHelper.buildOptionalInClause(healthIds, 'health_id', paramOffset);
285+
healthsClause = clause.replace('health_id', 'p.health_id');
286286
filterParams.push(...params);
287287
paramOffset += params.length;
288288
}
289289

290290
let categoriesClause = "";
291291
if (req.query.categories) {
292292
const categoryIds = (req.query.categories as string).split(",").filter(id => id.trim());
293-
const { clause, params } = SqlHelper.buildOptionalInClause(categoryIds, 'p.category_id', paramOffset);
294-
categoriesClause = clause;
293+
const { clause, params } = SqlHelper.buildOptionalInClause(categoryIds, 'category_id', paramOffset);
294+
categoriesClause = clause.replace('category_id', 'p.category_id');
295295
filterParams.push(...params);
296296
paramOffset += params.length;
297297
}
@@ -308,8 +308,8 @@ export default class ReportingProjectsController extends ReportingProjectsBase {
308308
let teamsClause = "";
309309
if (req.query.teams) {
310310
const teamIds = (req.query.teams as string).split(",").filter(id => id.trim());
311-
const { clause, params } = SqlHelper.buildOptionalInClause(teamIds, 'p.team_id', paramOffset);
312-
teamsClause = clause;
311+
const { clause, params } = SqlHelper.buildOptionalInClause(teamIds, 'team_id', paramOffset);
312+
teamsClause = clause.replace('team_id', 'p.team_id');
313313
filterParams.push(...params);
314314
paramOffset += params.length;
315315
}

‎worklenz-backend/src/shared/sql-helpers.ts‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export class SqlHelper {
112112

113113
conditions.forEach((condition, index) => {
114114
const conjunction = index === 0 ? "" : ` ${condition.conjunction || "AND"} `;
115-
115+
116116
if (condition.operator.toUpperCase() === "IN") {
117117
const values = Array.isArray(condition.value) ? condition.value : [condition.value];
118118
const { clause, params: inParams } = this.buildInClause(values, currentParam);
@@ -148,13 +148,13 @@ export class SqlHelper {
148148
} = {}
149149
): { clause: string; params: string[] } {
150150
const { caseSensitive = false, prefix = true, suffix = true } = options;
151-
151+
152152
let pattern = searchTerm;
153153
if (prefix) pattern = `%${pattern}`;
154154
if (suffix) pattern = `${pattern}%`;
155-
155+
156156
const operator = caseSensitive ? "LIKE" : "ILIKE";
157-
157+
158158
return {
159159
clause: `${field} ${operator} $${paramOffset}`,
160160
params: [pattern],
@@ -177,7 +177,7 @@ export class SqlHelper {
177177
const operator = caseSensitive ? "LIKE" : "ILIKE";
178178
const pattern = `%${searchTerm}%`;
179179
const clauses = fields.map(field => `${field} ${operator} $${paramOffset}`);
180-
180+
181181
return {
182182
clause: `(${clauses.join(" OR ")})`,
183183
params: [pattern],
@@ -223,14 +223,32 @@ export class SqlHelper {
223223

224224
/**
225225
* Escape identifier (table/column name) for secure query building
226+
* Supports both simple identifiers (e.g., "status_id") and qualified identifiers (e.g., "p.status_id")
226227
*/
227228
static escapeIdentifier(identifier: string): string {
229+
// Handle qualified identifiers (e.g., "p.status_id")
230+
if (identifier.includes('.')) {
231+
const segments = identifier.split('.');
232+
const escapedSegments = segments.map(segment => {
233+
const cleaned = segment.replace(/"/g, "");
234+
235+
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(cleaned)) {
236+
throw new Error(`Invalid identifier segment: ${segment}`);
237+
}
238+
239+
return `"${cleaned}"`;
240+
});
241+
242+
return escapedSegments.join('.');
243+
}
244+
245+
// Handle simple identifiers
228246
const cleaned = identifier.replace(/"/g, "");
229-
247+
230248
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(cleaned)) {
231249
throw new Error(`Invalid identifier: ${identifier}`);
232250
}
233-
251+
234252
return `"${cleaned}"`;
235253
}
236254

0 commit comments

Comments
 (0)