Skip to content

prevent SQL injection by using parameterized queries. #709

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.

Sign up for GitHub

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

Open
hitakshiA wants to merge 1 commit into
base: main
Choose a base branch
from
Open

prevent SQL injection by using parameterized queries. #709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
111 changes: 67 additions & 44 deletions src/fields/fields.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,18 @@ export class FieldsService {
);
}

const params = [];
if (requiredData.context) {
whereClause += ` OR context = '${requiredData.context}' AND "contextType" IS NULL`;
params.push(requiredData.context);
whereClause += ` OR context = $${params.length} AND "contextType" IS NULL`;
}

if (requiredData.contextType) {
whereClause += ` OR "contextType" = '${requiredData.contextType}'`;
params.push(requiredData.contextType);
whereClause += ` OR "contextType" = $${params.length}`;
}

const data = await this.getFieldData(whereClause);
const data = await this.getFieldData(whereClause, params);
Comment on lines +68 to +79
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep contextType scoped to the requested context.

When both filters are present, Line 76 turns the predicate into a standalone OR "contextType" = $n, so this query can pull fields from other contexts that happen to share the same contextType. That broadens getFormCustomField() beyond the requested schema.

🛠️ Suggested fix 
-      const params = [];
-      if (requiredData.context) {
-        params.push(requiredData.context);
-        whereClause += ` OR context = $${params.length} AND "contextType" IS NULL`;
-      }
-
-      if (requiredData.contextType) {
-        params.push(requiredData.contextType);
-        whereClause += ` OR "contextType" = $${params.length}`;
-      }
-
-      const data = await this.getFieldData(whereClause, params);
+      const params = [];
+      const clauses = ['(context IS NULL AND "contextType" IS NULL)'];
+
+      if (requiredData.context) {
+        params.push(requiredData.context);
+        clauses.push(`(context = $${params.length} AND "contextType" IS NULL)`);
+      }
+
+      if (requiredData.contextType) {
+        if (requiredData.context) {
+          params.push(requiredData.context, requiredData.contextType);
+          clauses.push(`(context = $${params.length - 1} AND "contextType" = $${params.length})`);
+        } else {
+          params.push(requiredData.contextType);
+          clauses.push(`("contextType" = $${params.length})`);
+        }
+      }
+
+      const data = await this.getFieldData(clauses.join(" OR "), params);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const params = [];
if (requiredData.context) {
whereClause += ` OR context = '${requiredData.context}' AND "contextType" IS NULL`;
params.push(requiredData.context);
whereClause += ` OR context = $${params.length} AND "contextType" IS NULL`;
}
if (requiredData.contextType) {
whereClause += ` OR "contextType" = '${requiredData.contextType}'`;
params.push(requiredData.contextType);
whereClause += ` OR "contextType" = $${params.length}`;
}
const data = await this.getFieldData(whereClause);
const data = await this.getFieldData(whereClause, params);
const params = [];
const clauses = ['(context IS NULL AND "contextType" IS NULL)'];
if (requiredData.context) {
params.push(requiredData.context);
clauses.push(`(context = $${params.length} AND "contextType" IS NULL)`);
}
if (requiredData.contextType) {
if (requiredData.context) {
params.push(requiredData.context, requiredData.contextType);
clauses.push(`(context = $${params.length - 1} AND "contextType" = $${params.length})`);
} else {
params.push(requiredData.contextType);
clauses.push(`("contextType" = $${params.length})`);
}
}
const data = await this.getFieldData(clauses.join(" OR "), params);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/fields/fields.service.ts` around lines 68 - 79, The whereClause currently
appends an independent OR clause for contextType which allows matching that
contextType across any context; instead, when both requiredData.context and
requiredData.contextType are provided, combine them into a single predicate so
contextType is scoped to that context (e.g. append OR (context = $i AND
"contextType" = $j) or build conditions via an array and join with proper
parentheses). Update the logic around whereClause/params in the same block
(references: whereClause, params, requiredData.context,
requiredData.contextType, getFieldData) so that the contextType clause is never
emitted standalone if a context value exists. Ensure parameter indices remain
correct when pushing combined conditions before calling this.getFieldData.

if (!data) {
return APIResponse.error(
response,
Expand Down Expand Up @@ -314,10 +317,10 @@ export class FieldsService {
};
}

async getFieldData(whereClause: any, tenantId?: string): Promise<any> {
async getFieldData(whereClause: any, params: any[] = [], tenantId?: string): Promise<any> {
const query = `select * from public."Fields" where ${whereClause}`;

const result = await this.fieldsRepository.query(query);
const result = await this.fieldsRepository.query(query, params);
if (!result) {
return false;
}
Expand Down Expand Up @@ -393,13 +396,13 @@ export class FieldsService {
storeWithoutControllingField.push(sourceFieldName["name"]);
}

const query = `SELECT "name", "value"
FROM public.${fieldsData.sourceDetails.table}
WHERE value = '${sourceFieldName["value"]}'
const query = `SELECT "name", "value"
FROM public.${fieldsData.sourceDetails.table}
WHERE value = $1
GROUP BY "name", "value"`;

const checkSourceData = await this.fieldsValuesRepository.query(
query
query, [sourceFieldName["value"]]
Comment on lines +399 to +405
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

These queries are still injectable through raw identifiers/fragments.

$n placeholders only protect literal values. The touched lines still splice sourceDetails.table / tableName / whereClause / order directly into the SQL text, and in this service those values are request-derived or persisted from earlier requests. The injection surface is still open even after parameterizing the value operands.

🔒 Hardening pattern 
+ private readonly ALLOWED_SOURCE_TABLES = new Set([
+   "state",
+   "district",
+   "block",
+   "village",
+   // add every supported source table here
+ ]);
+
+ private getSafeSourceTable(tableName: string): string {
+   if (!this.ALLOWED_SOURCE_TABLES.has(tableName)) {
+     throw new Error(`Unsupported source table: ${tableName}`);
+   }
+   return `"${tableName}"`;
+ }

Use that helper anywhere a table name is currently interpolated, and build WHERE / ORDER BY from validated enums instead of accepting raw SQL fragments.

Also applies to: 538-545, 700-724, 738-750, 1402-1403, 1501-1503

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/fields/fields.service.ts` around lines 399 - 405, The SQL concatenates
request-derived identifiers (e.g. sourceDetails.table, tableName, whereClause,
order) into raw queries used by fieldsValuesRepository.query, leaving SQL
injection risk; fix by validating and whitelisting table and column identifiers
(e.g. enforce sourceDetails.table against an allow-list or enum and map
where/order options to predefined clauses) or switch to a safe query builder
(e.g. knex/TypeORM QueryBuilder) instead of string concatenation, then pass only
literal values via $n parameters (e.g. the sourceFieldName["value"]); apply the
same validation/whitelisting approach to the other occurrences you flagged
(lines ~538-545, 700-724, 738-750, 1402-1403, 1501-1503) and replace any raw
fragment interpolation with mapped safe fragments or builder methods before
calling fieldsValuesRepository.query.

);

//If code is not exist in db
Expand Down Expand Up @@ -532,13 +535,13 @@ export class FieldsService {
}

// check options exits in source table column or not
const query = `SELECT "name", "value"
FROM public.${getSourceDetails.sourceDetails.table}
WHERE value = '${sourceFieldName["value"]}'
const query = `SELECT "name", "value"
FROM public.${getSourceDetails.sourceDetails.table}
WHERE value = $1
GROUP BY "name", "value"`;

const checkSourceData = await this.fieldsValuesRepository.query(
query
query, [sourceFieldName["value"]]
);

//If code is not exist in db
Expand Down Expand Up @@ -604,8 +607,8 @@ export class FieldsService {
}

// check options exits in fieldParams column or not
const query = `SELECT COUNT(*) FROM public."Fields" WHERE "fieldId"='${fieldId}' AND "fieldParams" -> 'options' @> '[{"value": "${sourceFieldName["value"]}"}]' `;
const checkSourceData = await this.fieldsRepository.query(query);
const query = `SELECT COUNT(*) FROM public."Fields" WHERE "fieldId"=$1 AND "fieldParams" -> 'options' @> $2::jsonb`;
const checkSourceData = await this.fieldsRepository.query(query, [fieldId, JSON.stringify([{ value: sourceFieldName["value"] }])]);

//If fields is not present then create a new options
if (checkSourceData[0].count == 0) {
Expand Down Expand Up @@ -694,18 +697,20 @@ export class FieldsService {
controllingfieldfk?: string,
dependsOn?: string
) {
const params = [name, value, createdBy];
let createSourceFields = `INSERT INTO public.${tableName} ("name", "value", "createdBy"`;

// Add controllingfieldfk to the columns if it is defined
if (controllingfieldfk !== undefined && controllingfieldfk !== "") {
createSourceFields += `, controllingfieldfk`;
}

createSourceFields += `) VALUES ('${name}', '${value}', '${createdBy}'`;
createSourceFields += `) VALUES ($1, $2, $3`;

// Add controllingfieldfk to the values if it is defined
if (controllingfieldfk !== undefined && controllingfieldfk !== "") {
createSourceFields += `, '${controllingfieldfk}'`;
params.push(controllingfieldfk);
createSourceFields += `, $${params.length}`;
}

createSourceFields += `);`;
Expand All @@ -716,7 +721,7 @@ export class FieldsService {

//Insert data into source table
const checkSourceData = await this.fieldsValuesRepository.query(
createSourceFields
createSourceFields, params
);
if (checkSourceData.length == 0) {
return false;
Expand All @@ -730,16 +735,19 @@ export class FieldsService {
updatedBy: string,
controllingfieldfk?: string
) {
let updateSourceDetails = `UPDATE public.${tableName} SET "name"='${name}',"updatedBy"='${updatedBy}'`;
const params = [name, updatedBy];
let updateSourceDetails = `UPDATE public.${tableName} SET "name"=$1,"updatedBy"=$2`;

if (controllingfieldfk !== undefined) {
updateSourceDetails += `, controllingfieldfk='${controllingfieldfk}'`;
params.push(controllingfieldfk);
updateSourceDetails += `, controllingfieldfk=$${params.length}`;
}

updateSourceDetails += ` WHERE value='${value}';`;
params.push(value);
updateSourceDetails += ` WHERE value=$${params.length};`;

const updateSourceData = await this.fieldsValuesRepository.query(
updateSourceDetails
updateSourceDetails, params
);
if (updateSourceData.length == 0) {
return false;
Expand Down Expand Up @@ -797,9 +805,14 @@ export class FieldsService {
const fieldKeys = this.fieldsRepository.metadata.columns.map(
(column) => column.propertyName
);
let tenantCond = tenantId
? `"tenantId" = '${tenantId}'`
: `"tenantId" IS NULL`;
const params = [];
let tenantCond;
if (tenantId) {
params.push(tenantId);
tenantCond = `"tenantId" = $${params.length}`;
} else {
tenantCond = `"tenantId" IS NULL`;
}
let whereClause = tenantCond;
if (filters && Object.keys(filters).length > 0) {
Object.entries(filters).forEach(([key, value]) => {
Expand All @@ -808,9 +821,11 @@ export class FieldsService {
key === "context" &&
(value === "USERS" || value === "COHORT")
) {
whereClause += ` AND "context" = '${value}'`;
params.push(value);
whereClause += ` AND "context" = $${params.length}`;
} else {
whereClause += ` AND "${key}" = '${value}'`;
params.push(value);
whereClause += ` AND "${key}" = $${params.length}`;
}
} else {
return APIResponse.error(
Expand All @@ -824,7 +839,7 @@ export class FieldsService {
});
}

const fieldData = await this.getFieldData(whereClause);
const fieldData = await this.getFieldData(whereClause, params);
if (!fieldData.length) {
return APIResponse.error(
response,
Expand Down Expand Up @@ -1381,10 +1396,11 @@ export class FieldsService {
//Delete data from source table
if (getField?.sourceDetails?.source == "table") {
const whereCond = requiredData.option
? `WHERE "value"='${requiredData.option}'`
? `WHERE "value"=$1`
: "";
const deleteParams = requiredData.option ? [requiredData.option] : [];
const query = `DELETE FROM public.${getField?.sourceDetails?.table} ${whereCond}`;
const [_, affectedRow] = await this.fieldsRepository.query(query);
const [_, affectedRow] = await this.fieldsRepository.query(query, deleteParams);

if (affectedRow === 0) {
return await APIResponse.error(
Expand All @@ -1400,8 +1416,8 @@ export class FieldsService {
//Delete data from fieldParams column
if (getField?.sourceDetails?.source == "fieldparams") {
// check options exits in fieldParams column or not
const query = `SELECT * FROM public."Fields" WHERE "fieldId"='${getField.fieldId}' AND "fieldParams" -> 'options' @> '[{"value": "${removeOption}"}]' `;
const checkSourceData = await this.fieldsRepository.query(query);
const query = `SELECT * FROM public."Fields" WHERE "fieldId"=$1 AND "fieldParams" -> 'options' @> $2::jsonb`;
const checkSourceData = await this.fieldsRepository.query(query, [getField.fieldId, JSON.stringify([{ value: removeOption }])]);

if (checkSourceData.length > 0) {
let fieldParamsOptions = checkSourceData[0].fieldParams.options;
Expand Down Expand Up @@ -1466,6 +1482,7 @@ export class FieldsService {
const offsetCond = offset ? `offset ${offset}` : "";
const limitCond = limit ? `limit ${limit}` : "";
const conditions = [];
const params = [];

if (whereClause) {
conditions.push(`${whereClause}`);
Expand All @@ -1475,14 +1492,15 @@ export class FieldsService {
conditions.push(`is_active=1`);

if (optionSelected) {
conditions.push(`"${tableName}_name" ILike '%${optionSelected}%'`);
params.push(`%${optionSelected}%`);
conditions.push(`"${tableName}_name" ILike $${params.length}`);
}

const whereCond = conditions.length ? `WHERE ${conditions.join(" AND ")}` : "";

const query = `SELECT *,COUNT(*) OVER() AS total_count FROM public."${tableName}" ${whereCond} ${orderCond} ${offsetCond} ${limitCond}`;

const result = await this.fieldsRepository.query(query);
const result = await this.fieldsRepository.query(query, params);
if (!result) {
return null;
}
Expand Down Expand Up @@ -1603,7 +1621,7 @@ export class FieldsService {
}

async filterUserUsingCustomFields(context: string, stateDistBlockData: any) {
const searchKey = [];
const params = [];
let whereCondition = ` WHERE `;
let index = 0;
const tableName = "";
Expand All @@ -1617,36 +1635,41 @@ export class FieldsService {
joinCond = ``;
}

const searchKeys = Object.keys(stateDistBlockData);
for (const [key, value] of Object.entries(stateDistBlockData)) {
searchKey.push(`'${key}'`);
if (index > 0) {
whereCondition += ` AND `;
}

// using the ?| array[] operator to search for both single and multiple values in a JSONB column.
whereCondition += `fields->'${key}' ?| array[${(Array.isArray(value)
? value
: [value]
)
.map((v) => `'${v}'`)
.join(",")}]`;
params.push(key);
const keyParam = params.length;
const values = Array.isArray(value) ? value : [value];
params.push(values);
const valParam = params.length;
whereCondition += `fields->$${keyParam} ?| $${valParam}::text[]`;
index++;
}

params.push(searchKeys);
const searchKeyParam = params.length;
params.push(context);
const contextParam = params.length;

const query = `WITH user_fields AS (
SELECT
fv."itemId",
jsonb_object_agg(f."name", fv."value") AS fields
FROM "FieldValues" fv
JOIN "Fields" f ON fv."fieldId" = f."fieldId"
${joinCond}
WHERE f."name" IN (${searchKey}) AND (f.context IN('${context}', 'NULL', 'null', '') OR f.context IS NULL)
WHERE f."name" = ANY($${searchKeyParam}) AND (f.context IN($${contextParam}, 'NULL', 'null', '') OR f.context IS NULL)
GROUP BY fv."itemId"
)
SELECT "itemId"
FROM user_fields ${whereCondition}`;

const queryData = await this.fieldsValuesRepository.query(query);
const queryData = await this.fieldsValuesRepository.query(query, params);
const result =
queryData.length > 0 ? queryData.map((item) => item.itemId) : null;
return result;
Expand Down
4 changes: 2 additions & 2 deletions src/tenant/tenant.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ export class TenantService {

// Fetch roles only for child tenants (skip for parent tenants)
if (tenantData.parentId) {
let query = `SELECT * FROM public."Roles" WHERE "tenantId" = '${tenantData.tenantId}'`;
let getRole = await this.tenantRepository.query(query);
let query = `SELECT * FROM public."Roles" WHERE "tenantId" = $1`;
let getRole = await this.tenantRepository.query(query, [tenantData.tenantId]);

if (getRole.length === 0) {
// fallback if tenant-specific roles not found
Expand Down
4 changes: 2 additions & 2 deletions src/user/user.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2131,10 +2131,10 @@ export class UserService {
for (const mapData of tenantCohortRoleMapping) {
if (mapData.cohortIds) {
for (const cohortIds of mapData.cohortIds) {
let query = `SELECT * FROM public."CohortAcademicYear" WHERE "cohortId"= '${cohortIds}' AND "academicYearId" = '${academicYearId}'`;
let query = `SELECT * FROM public."CohortAcademicYear" WHERE "cohortId"= $1 AND "academicYearId" = $2`;

let getCohortAcademicYearId = await this.usersRepository.query(
query
query, [cohortIds, academicYearId]
);

// will add data only if cohort is found with academic year
Expand Down