fix(server): prevent SQL injection via sortOrder in DynamicListing (GHSA-hcp2-qqg6-jjpm)
Validate sortOrder against an allowlist at the DTO layer, normalize the direction centrally in DynamicFilterSortBy.buildQuery, and re-sanitize inside every orderByRaw modifier so attacker-controlled SQL cannot reach the ORDER BY clause. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -11,6 +11,7 @@ import { TenantBaseModel } from '@/modules/System/models/TenantBaseModel';
|
|||||||
import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.decorator';
|
import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.decorator';
|
||||||
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
||||||
import { BillMeta } from './Bill.meta';
|
import { BillMeta } from './Bill.meta';
|
||||||
|
import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
|
||||||
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
||||||
import { BillDefaultViews } from '../Bills.constants';
|
import { BillDefaultViews } from '../Bills.constants';
|
||||||
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
||||||
@@ -407,7 +408,8 @@ export class Bill extends TenantBaseModel {
|
|||||||
* Sort the bills by full-payment bills.
|
* Sort the bills by full-payment bills.
|
||||||
*/
|
*/
|
||||||
sortByStatus(query, order) {
|
sortByStatus(query, order) {
|
||||||
query.orderByRaw(`PAYMENT_AMOUNT = AMOUNT ${order}`);
|
const dir = sanitizeSortDirection(order);
|
||||||
|
query.orderByRaw(`PAYMENT_AMOUNT = AMOUNT ${dir}`);
|
||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/Inje
|
|||||||
import { ItemEntry } from '@/modules/TransactionItemEntry/models/ItemEntry';
|
import { ItemEntry } from '@/modules/TransactionItemEntry/models/ItemEntry';
|
||||||
import { Warehouse } from '@/modules/Warehouses/models/Warehouse.model';
|
import { Warehouse } from '@/modules/Warehouses/models/Warehouse.model';
|
||||||
import { CreditNoteMeta } from './CreditNote.meta';
|
import { CreditNoteMeta } from './CreditNote.meta';
|
||||||
|
import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
|
||||||
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
||||||
import { CreditNoteDefaultViews } from '../constants';
|
import { CreditNoteDefaultViews } from '../constants';
|
||||||
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
||||||
@@ -277,8 +278,9 @@ export class CreditNote extends TenantBaseModel {
|
|||||||
*
|
*
|
||||||
*/
|
*/
|
||||||
sortByStatus(query, order) {
|
sortByStatus(query, order) {
|
||||||
|
const dir = sanitizeSortDirection(order);
|
||||||
query.orderByRaw(
|
query.orderByRaw(
|
||||||
`COALESCE(REFUNDED_AMOUNT) + COALESCE(INVOICES_AMOUNT) = COALESCE(AMOUNT) ${order}`,
|
`COALESCE(REFUNDED_AMOUNT) + COALESCE(INVOICES_AMOUNT) = COALESCE(AMOUNT) ${dir}`,
|
||||||
);
|
);
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { FIELD_TYPE } from './constants';
|
import { FIELD_TYPE } from './constants';
|
||||||
import { DynamicFilterRoleAbstractor } from './DynamicFilterRoleAbstractor';
|
import { DynamicFilterRoleAbstractor } from './DynamicFilterRoleAbstractor';
|
||||||
|
import { sanitizeSortDirection } from './sanitizeSortDirection';
|
||||||
|
|
||||||
interface ISortRole {
|
interface ISortRole {
|
||||||
fieldKey: string;
|
fieldKey: string;
|
||||||
@@ -67,17 +68,18 @@ export class DynamicFilterSortBy extends DynamicFilterRoleAbstractor {
|
|||||||
public buildQuery = () => {
|
public buildQuery = () => {
|
||||||
const field = this.model.getField(this.sortRole.fieldKey);
|
const field = this.model.getField(this.sortRole.fieldKey);
|
||||||
const comparatorColumn = this.getFieldComparatorColumn(field);
|
const comparatorColumn = this.getFieldComparatorColumn(field);
|
||||||
|
const safeOrder = sanitizeSortDirection(this.sortRole.order);
|
||||||
|
|
||||||
// Sort custom query.
|
// Sort custom query.
|
||||||
if (typeof field.sortCustomQuery !== 'undefined') {
|
if (typeof field.sortCustomQuery !== 'undefined') {
|
||||||
return (builder) => {
|
return (builder) => {
|
||||||
field.sortCustomQuery(builder, this.sortRole);
|
field.sortCustomQuery(builder, { ...this.sortRole, order: safeOrder });
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
return (builder) => {
|
return (builder) => {
|
||||||
if (this.sortRole.fieldKey) {
|
if (this.sortRole.fieldKey) {
|
||||||
builder.orderBy(`${comparatorColumn}`, this.sortRole.order);
|
builder.orderBy(`${comparatorColumn}`, safeOrder);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -0,0 +1,8 @@
|
|||||||
|
/**
|
||||||
|
* Normalises an arbitrary `sortOrder` value to a SQL-safe direction.
|
||||||
|
* Returns 'DESC' only on an explicit case-insensitive match; otherwise 'ASC'.
|
||||||
|
* Used to defuse `orderByRaw` interpolation in dynamic listing modifiers.
|
||||||
|
*/
|
||||||
|
export function sanitizeSortDirection(order: unknown): 'ASC' | 'DESC' {
|
||||||
|
return String(order ?? '').toUpperCase() === 'DESC' ? 'DESC' : 'ASC';
|
||||||
|
}
|
||||||
@@ -1,6 +1,6 @@
|
|||||||
import { ToNumber } from '@/common/decorators/Validators';
|
import { ToNumber } from '@/common/decorators/Validators';
|
||||||
import { ApiPropertyOptional } from '@nestjs/swagger';
|
import { ApiPropertyOptional } from '@nestjs/swagger';
|
||||||
import { IsArray, IsInt, IsOptional, IsString } from 'class-validator';
|
import { IsArray, IsIn, IsInt, IsOptional, IsString } from 'class-validator';
|
||||||
import { IFilterRole, ISortOrder } from '../DynamicFilter/DynamicFilter.types';
|
import { IFilterRole, ISortOrder } from '../DynamicFilter/DynamicFilter.types';
|
||||||
|
|
||||||
export class DynamicFilterQueryDto {
|
export class DynamicFilterQueryDto {
|
||||||
@@ -32,7 +32,7 @@ export class DynamicFilterQueryDto {
|
|||||||
columnSortBy: string;
|
columnSortBy: string;
|
||||||
|
|
||||||
@ApiPropertyOptional({ description: 'Sort order (asc/desc)', type: String })
|
@ApiPropertyOptional({ description: 'Sort order (asc/desc)', type: String })
|
||||||
@IsString()
|
@IsIn(['ASC', 'DESC', 'asc', 'desc'])
|
||||||
@IsOptional()
|
@IsOptional()
|
||||||
sortOrder: ISortOrder;
|
sortOrder: ISortOrder;
|
||||||
|
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import { TenantBaseModel } from '@/modules/System/models/TenantBaseModel';
|
|||||||
import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.decorator';
|
import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.decorator';
|
||||||
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
||||||
import { ManualJournalMeta } from './ManualJournal.meta';
|
import { ManualJournalMeta } from './ManualJournal.meta';
|
||||||
|
import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
|
||||||
import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
|
import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
|
||||||
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
||||||
import { ManualJournalDefaultViews } from '../constants';
|
import { ManualJournalDefaultViews } from '../constants';
|
||||||
@@ -80,7 +81,8 @@ export class ManualJournal extends TenantBaseModel {
|
|||||||
* Sort by status query.
|
* Sort by status query.
|
||||||
*/
|
*/
|
||||||
sortByStatus(query, order) {
|
sortByStatus(query, order) {
|
||||||
query.orderByRaw(`PUBLISHED_AT IS NULL ${order}`);
|
const dir = sanitizeSortDirection(order);
|
||||||
|
query.orderByRaw(`PUBLISHED_AT IS NULL ${dir}`);
|
||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.dec
|
|||||||
import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
|
import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
|
||||||
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
||||||
import { SaleEstimateMeta } from './SaleEstimate.meta';
|
import { SaleEstimateMeta } from './SaleEstimate.meta';
|
||||||
|
import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
|
||||||
import { ItemEntry } from '@/modules/TransactionItemEntry/models/ItemEntry';
|
import { ItemEntry } from '@/modules/TransactionItemEntry/models/ItemEntry';
|
||||||
import { Document } from '@/modules/ChromiumlyTenancy/models/Document';
|
import { Document } from '@/modules/ChromiumlyTenancy/models/Document';
|
||||||
import { Customer } from '@/modules/Customers/models/Customer';
|
import { Customer } from '@/modules/Customers/models/Customer';
|
||||||
@@ -250,7 +251,8 @@ export class SaleEstimate extends TenantBaseModel {
|
|||||||
* Sorting the estimates orders by delivery status.
|
* Sorting the estimates orders by delivery status.
|
||||||
*/
|
*/
|
||||||
orderByStatus(query, order) {
|
orderByStatus(query, order) {
|
||||||
query.orderByRaw(`delivered_at is null ${order}`);
|
const dir = sanitizeSortDirection(order);
|
||||||
|
query.orderByRaw(`delivered_at is null ${dir}`);
|
||||||
},
|
},
|
||||||
/**
|
/**
|
||||||
* Filtering the estimates oreders by status field.
|
* Filtering the estimates oreders by status field.
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import { Document } from '@/modules/ChromiumlyTenancy/models/Document';
|
|||||||
import { DiscountType } from '@/common/types/Discount';
|
import { DiscountType } from '@/common/types/Discount';
|
||||||
import { Account } from '@/modules/Accounts/models/Account.model';
|
import { Account } from '@/modules/Accounts/models/Account.model';
|
||||||
import { ISearchRole } from '@/modules/DynamicListing/DynamicFilter/DynamicFilter.types';
|
import { ISearchRole } from '@/modules/DynamicListing/DynamicFilter/DynamicFilter.types';
|
||||||
|
import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
|
||||||
import { TenantBaseModel } from '@/modules/System/models/TenantBaseModel';
|
import { TenantBaseModel } from '@/modules/System/models/TenantBaseModel';
|
||||||
import { TransactionPaymentServiceEntry } from '@/modules/PaymentServices/models/TransactionPaymentServiceEntry.model';
|
import { TransactionPaymentServiceEntry } from '@/modules/PaymentServices/models/TransactionPaymentServiceEntry.model';
|
||||||
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
||||||
@@ -417,14 +418,16 @@ export class SaleInvoice extends TenantBaseModel {
|
|||||||
* Sort the sale invoices by full-payment invoices.
|
* Sort the sale invoices by full-payment invoices.
|
||||||
*/
|
*/
|
||||||
sortByStatus(query, order) {
|
sortByStatus(query, order) {
|
||||||
query.orderByRaw(`PAYMENT_AMOUNT = BALANCE ${order}`);
|
const dir = sanitizeSortDirection(order);
|
||||||
|
query.orderByRaw(`PAYMENT_AMOUNT = BALANCE ${dir}`);
|
||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Sort the sale invoices by the due amount.
|
* Sort the sale invoices by the due amount.
|
||||||
*/
|
*/
|
||||||
sortByDueAmount(query, order) {
|
sortByDueAmount(query, order) {
|
||||||
query.orderByRaw(`BALANCE - PAYMENT_AMOUNT ${order}`);
|
const dir = sanitizeSortDirection(order);
|
||||||
|
query.orderByRaw(`BALANCE - PAYMENT_AMOUNT ${dir}`);
|
||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
|
|||||||
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
||||||
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
||||||
import { SaleReceiptMeta } from './SaleReceipt.meta';
|
import { SaleReceiptMeta } from './SaleReceipt.meta';
|
||||||
|
import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
|
||||||
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
||||||
import { SaleReceiptDefaultViews } from '../constants';
|
import { SaleReceiptDefaultViews } from '../constants';
|
||||||
|
|
||||||
@@ -238,7 +239,8 @@ export class SaleReceipt extends ExtendedModel {
|
|||||||
* Sorting the receipts order by status.
|
* Sorting the receipts order by status.
|
||||||
*/
|
*/
|
||||||
sortByStatus(query, order) {
|
sortByStatus(query, order) {
|
||||||
query.orderByRaw(`CLOSED_AT IS NULL ${order}`);
|
const dir = sanitizeSortDirection(order);
|
||||||
|
query.orderByRaw(`CLOSED_AT IS NULL ${dir}`);
|
||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.dec
|
|||||||
import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
|
import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
|
||||||
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
|
||||||
import { VendorCreditMeta } from './VendorCredit.meta';
|
import { VendorCreditMeta } from './VendorCredit.meta';
|
||||||
|
import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
|
||||||
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
|
||||||
import { VendorCreditDefaultViews } from '../constants';
|
import { VendorCreditDefaultViews } from '../constants';
|
||||||
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
|
||||||
@@ -198,8 +199,9 @@ export class VendorCredit extends TenantBaseModel {
|
|||||||
*
|
*
|
||||||
*/
|
*/
|
||||||
sortByStatus(query, order) {
|
sortByStatus(query, order) {
|
||||||
|
const dir = sanitizeSortDirection(order);
|
||||||
query.orderByRaw(
|
query.orderByRaw(
|
||||||
`COALESCE(REFUNDED_AMOUNT) + COALESCE(INVOICED_AMOUNT) = COALESCE(AMOUNT) ${order}`,
|
`COALESCE(REFUNDED_AMOUNT) + COALESCE(INVOICED_AMOUNT) = COALESCE(AMOUNT) ${dir}`,
|
||||||
);
|
);
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user