Merge pull request #1097 from bigcapitalhq/fix/sortorder-sql-injection

fix(server): prevent SQL injection via sortOrder in DynamicListing (GHSA-hcp2-qqg6-jjpm)

packages/server/src/modules/Bills/models/Bill.ts

@@ -11,6 +11,7 @@ import { TenantBaseModel } from '@/modules/System/models/TenantBaseModel';
 import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.decorator';
 import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
 import { BillMeta } from './Bill.meta';
+import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
 import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
 import { BillDefaultViews } from '../Bills.constants';
 import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
@@ -407,7 +408,8 @@ export class Bill extends TenantBaseModel {
        * Sort the bills by full-payment bills.
        */
       sortByStatus(query, order) {
-        query.orderByRaw(`PAYMENT_AMOUNT = AMOUNT ${order}`);
+        const dir = sanitizeSortDirection(order);
+        query.orderByRaw(`PAYMENT_AMOUNT = AMOUNT ${dir}`);
       },
 
       /**

packages/server/src/modules/CreditNotes/models/CreditNote.ts

@@ -9,6 +9,7 @@ import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/Inje
 import { ItemEntry } from '@/modules/TransactionItemEntry/models/ItemEntry';
 import { Warehouse } from '@/modules/Warehouses/models/Warehouse.model';
 import { CreditNoteMeta } from './CreditNote.meta';
+import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
 import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
 import { CreditNoteDefaultViews } from '../constants';
 import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
@@ -277,8 +278,9 @@ export class CreditNote extends TenantBaseModel {
        *
        */
       sortByStatus(query, order) {
+        const dir = sanitizeSortDirection(order);
         query.orderByRaw(
-          `COALESCE(REFUNDED_AMOUNT) + COALESCE(INVOICES_AMOUNT) = COALESCE(AMOUNT) ${order}`,
+          `COALESCE(REFUNDED_AMOUNT) + COALESCE(INVOICES_AMOUNT) = COALESCE(AMOUNT) ${dir}`,
         );
       },
     };

packages/server/src/modules/DynamicListing/DynamicFilter/DynamicFilterSortBy.ts

@@ -1,5 +1,6 @@
 import { FIELD_TYPE } from './constants';
 import { DynamicFilterRoleAbstractor } from './DynamicFilterRoleAbstractor';
+import { sanitizeSortDirection } from './sanitizeSortDirection';
 
 interface ISortRole {
   fieldKey: string;
@@ -67,17 +68,18 @@ export class DynamicFilterSortBy extends DynamicFilterRoleAbstractor {
   public buildQuery = () => {
     const field = this.model.getField(this.sortRole.fieldKey);
     const comparatorColumn = this.getFieldComparatorColumn(field);
+    const safeOrder = sanitizeSortDirection(this.sortRole.order);
 
     // Sort custom query.
     if (typeof field.sortCustomQuery !== 'undefined') {
       return (builder) => {
-        field.sortCustomQuery(builder, this.sortRole);
+        field.sortCustomQuery(builder, { ...this.sortRole, order: safeOrder });
       };
     }
 
     return (builder) => {
       if (this.sortRole.fieldKey) {
-        builder.orderBy(`${comparatorColumn}`, this.sortRole.order);
+        builder.orderBy(`${comparatorColumn}`, safeOrder);
       }
     };
   };

packages/server/src/modules/DynamicListing/DynamicFilter/sanitizeSortDirection.ts

@@ -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';
+}

packages/server/src/modules/DynamicListing/dtos/DynamicFilterQuery.dto.ts

@@ -1,6 +1,6 @@
 import { ToNumber } from '@/common/decorators/Validators';
 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';
 
 export class DynamicFilterQueryDto {
@@ -32,7 +32,7 @@ export class DynamicFilterQueryDto {
   columnSortBy: string;
 
   @ApiPropertyOptional({ description: 'Sort order (asc/desc)', type: String })
-  @IsString()
+  @IsIn(['ASC', 'DESC', 'asc', 'desc'])
   @IsOptional()
   sortOrder: ISortOrder;
 

packages/server/src/modules/ManualJournals/models/ManualJournal.ts

@@ -5,6 +5,7 @@ import { TenantBaseModel } from '@/modules/System/models/TenantBaseModel';
 import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.decorator';
 import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
 import { ManualJournalMeta } from './ManualJournal.meta';
+import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
 import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
 import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
 import { ManualJournalDefaultViews } from '../constants';
@@ -80,7 +81,8 @@ export class ManualJournal extends TenantBaseModel {
        * Sort by status query.
        */
       sortByStatus(query, order) {
-        query.orderByRaw(`PUBLISHED_AT IS NULL ${order}`);
+        const dir = sanitizeSortDirection(order);
+        query.orderByRaw(`PUBLISHED_AT IS NULL ${dir}`);
       },
 
       /**

packages/server/src/modules/SaleEstimates/models/SaleEstimate.ts

@@ -6,6 +6,7 @@ import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.dec
 import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
 import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
 import { SaleEstimateMeta } from './SaleEstimate.meta';
+import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
 import { ItemEntry } from '@/modules/TransactionItemEntry/models/ItemEntry';
 import { Document } from '@/modules/ChromiumlyTenancy/models/Document';
 import { Customer } from '@/modules/Customers/models/Customer';
@@ -250,7 +251,8 @@ export class SaleEstimate extends TenantBaseModel {
        * Sorting the estimates orders by delivery status.
        */
       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.

packages/server/src/modules/SaleInvoices/models/SaleInvoice.ts

@@ -10,6 +10,7 @@ import { Document } from '@/modules/ChromiumlyTenancy/models/Document';
 import { DiscountType } from '@/common/types/Discount';
 import { Account } from '@/modules/Accounts/models/Account.model';
 import { ISearchRole } from '@/modules/DynamicListing/DynamicFilter/DynamicFilter.types';
+import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
 import { TenantBaseModel } from '@/modules/System/models/TenantBaseModel';
 import { TransactionPaymentServiceEntry } from '@/modules/PaymentServices/models/TransactionPaymentServiceEntry.model';
 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.
        */
       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.
        */
       sortByDueAmount(query, order) {
-        query.orderByRaw(`BALANCE - PAYMENT_AMOUNT ${order}`);
+        const dir = sanitizeSortDirection(order);
+        query.orderByRaw(`BALANCE - PAYMENT_AMOUNT ${dir}`);
       },
 
       /**

packages/server/src/modules/SaleReceipts/models/SaleReceipt.ts

@@ -18,6 +18,7 @@ import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
 import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
 import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
 import { SaleReceiptMeta } from './SaleReceipt.meta';
+import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
 import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
 import { SaleReceiptDefaultViews } from '../constants';
 
@@ -238,7 +239,8 @@ export class SaleReceipt extends ExtendedModel {
        * Sorting the receipts order by status.
        */
       sortByStatus(query, order) {
-        query.orderByRaw(`CLOSED_AT IS NULL ${order}`);
+        const dir = sanitizeSortDirection(order);
+        query.orderByRaw(`CLOSED_AT IS NULL ${dir}`);
       },
 
       /**

packages/server/src/modules/VendorCredit/models/VendorCredit.ts

@@ -9,6 +9,7 @@ import { ExportableModel } from '@/modules/Export/decorators/ExportableModel.dec
 import { ImportableModel } from '@/modules/Import/decorators/Import.decorator';
 import { InjectModelMeta } from '@/modules/Tenancy/TenancyModels/decorators/InjectModelMeta.decorator';
 import { VendorCreditMeta } from './VendorCredit.meta';
+import { sanitizeSortDirection } from '@/modules/DynamicListing/DynamicFilter/sanitizeSortDirection';
 import { InjectModelDefaultViews } from '@/modules/Views/decorators/InjectModelDefaultViews.decorator';
 import { VendorCreditDefaultViews } from '../constants';
 import { InjectAttachable } from '@/modules/Attachments/decorators/InjectAttachable.decorator';
@@ -198,8 +199,9 @@ export class VendorCredit extends TenantBaseModel {
        *
        */
       sortByStatus(query, order) {
+        const dir = sanitizeSortDirection(order);
         query.orderByRaw(
-          `COALESCE(REFUNDED_AMOUNT) + COALESCE(INVOICED_AMOUNT) = COALESCE(AMOUNT) ${order}`,
+          `COALESCE(REFUNDED_AMOUNT) + COALESCE(INVOICED_AMOUNT) = COALESCE(AMOUNT) ${dir}`,
         );
       },
     };