Enhance analytics API with SQL string handling and validation improvements

- Added `sqlstring` package for safer SQL query construction across various analytics endpoints.
- Updated `getFunnel`, `getUserSessionCount`, and other functions to utilize `SqlString.escape` for parameterized inputs, enhancing security against SQL injection.
- Refactored input validation by replacing the `sql-sanitization` module with a new `query-validation` module, improving clarity and maintainability of parameter handling.
- Removed the deprecated `sql-sanitization.ts` file to streamline the codebase.

server/package-lock.json

@@ -23,6 +23,7 @@
         "node-cron": "^3.0.3",
         "pg": "^8.13.3",
         "postgres": "^3.4.5",
+        "sqlstring": "^2.3.3",
         "stripe": "^17.7.0",
         "ua-parser-js": "^2.0.0",
         "undici": "^7.3.0",
@@ -33,6 +34,7 @@
         "@types/node": "^20.10.0",
         "@types/node-cron": "^3.0.11",
         "@types/pg": "^8.11.11",
+        "@types/sqlstring": "^2.3.2",
         "drizzle-kit": "^0.30.5",
         "ts-node-dev": "^2.0.0",
         "tsx": "^4.19.3",
@@ -1348,6 +1350,12 @@
         "node": ">=12"
       }
     },
+    "node_modules/@types/sqlstring": {
+      "version": "2.3.2",
+      "resolved": "https://registry.npmjs.org/@types/sqlstring/-/sqlstring-2.3.2.tgz",
+      "integrity": "sha512-lVRe4Iz9UNgiHelKVo8QlC8fb5nfY8+p+jNQNE+UVsuuVlQnWhyWmQ/wF5pE8Ys6TdjfVpqTG9O9i2vi6E0+Sg==",
+      "dev": true
+    },
     "node_modules/@types/strip-bom": {
       "version": "3.0.0",
       "resolved": "https://registry.npmjs.org/@types/strip-bom/-/strip-bom-3.0.0.tgz",
@@ -3608,6 +3616,14 @@
         "node": ">= 10.x"
       }
     },
+    "node_modules/sqlstring": {
+      "version": "2.3.3",
+      "resolved": "https://registry.npmjs.org/sqlstring/-/sqlstring-2.3.3.tgz",
+      "integrity": "sha512-qC9iz2FlN7DQl3+wjwn3802RTyjCx7sDvfQEXchwa6CWOx07/WVfh91gBmQ9fahw8snwGEWU3xGzOt4tFyHLxg==",
+      "engines": {
+        "node": ">= 0.6"
+      }
+    },
     "node_modules/statuses": {
       "version": "2.0.1",
       "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz",

server/package.json

@@ -31,6 +31,7 @@
     "node-cron": "^3.0.3",
     "pg": "^8.13.3",
     "postgres": "^3.4.5",
+    "sqlstring": "^2.3.3",
     "stripe": "^17.7.0",
     "ua-parser-js": "^2.0.0",
     "undici": "^7.3.0",
@@ -41,6 +42,7 @@
     "@types/node": "^20.10.0",
     "@types/node-cron": "^3.0.11",
     "@types/pg": "^8.11.11",
+    "@types/sqlstring": "^2.3.2",
     "drizzle-kit": "^0.30.5",
     "ts-node-dev": "^2.0.0",
     "tsx": "^4.19.3",

server/src/api/analytics/getFunnel.ts

@@ -7,6 +7,7 @@ import {
   getFilterStatement,
 } from "./utils.js";
 import { getUserHasAccessToSitePublic } from "../../lib/auth-utils.js";
+import SqlString from "sqlstring";
 
 type FunnelStep = {
   value: string;
@@ -77,9 +78,11 @@ export async function getFunnel(
     // Build conditional statements for each step
     const stepConditions = steps.map((step) => {
       if (step.type === "page") {
-        return `pathname = '${step.value}'`;
+        return `pathname = ${SqlString.escape(step.value)}`;
       } else {
-        return `type = 'custom_event' AND event_name = '${step.value}'`;
+        return `type = 'custom_event' AND event_name = ${SqlString.escape(
+          step.value
+        )}`;
       }
     });
 
@@ -138,7 +141,7 @@ export async function getFunnel(
           (step, index) => `
           SELECT
             ${index + 1} as step_number,
-            '${step.name || step.value}' as step_name,
+            ${SqlString.escape(step.name || step.value)} as step_name,
             count(DISTINCT user_id) as visitors
           FROM Step${index + 1}
         `

server/src/api/analytics/getOverviewBucketed.ts

@@ -6,7 +6,7 @@ import {
   processResults,
 } from "./utils.js";
 import { getUserHasAccessToSitePublic } from "../../lib/auth-utils.js";
-import { sanitizeTimeStatementFillParams } from "./sql-sanitziation.js";
+import { validateTimeStatementFillParams } from "./query-validation.js";
 
 const TimeBucketToFn = {
   minute: "toStartOfMinute",
@@ -42,7 +42,7 @@ function getTimeStatementFill(
   },
   bucket: TimeBucket
 ) {
-  const { params, bucket: validatedBucket } = sanitizeTimeStatementFillParams(
+  const { params, bucket: validatedBucket } = validateTimeStatementFillParams(
     { date, pastMinutes },
     bucket
   );

server/src/api/analytics/getSingleCol.ts

@@ -38,7 +38,6 @@ type GetSingleColResponse = {
 const getQuery = (request: FastifyRequest<GenericRequest>) => {
   const { startDate, endDate, timezone, filters, parameter, limit, minutes } =
     request.query;
-  const site = request.params.site;
 
   const filterStatement = getFilterStatement(filters);
   const timeStatement = getTimeStatement(
@@ -49,6 +48,10 @@ const getQuery = (request: FastifyRequest<GenericRequest>) => {
         }
   );
 
+  if (typeof limit !== "number") {
+    throw new Error("Limit must be a number");
+  }
+
   const percentageStatement = `ROUND(
           COUNT(distinct(session_id)) * 100.0 / SUM(COUNT(distinct(session_id))) OVER (),
           2

server/src/api/analytics/getUserSessionCount.ts

@@ -2,6 +2,7 @@ import { FastifyReply, FastifyRequest } from "fastify";
 import clickhouse from "../../db/clickhouse/clickhouse.js";
 import { processResults } from "./utils.js";
 import { getUserHasAccessToSitePublic } from "../../lib/auth-utils.js";
+import SqlString from "sqlstring";
 
 export interface GetUserSessionCountRequest {
   Params: {
@@ -36,7 +37,7 @@ export async function getUserSessionCount(
 
   const query = `
     SELECT
-      toDate(session_start, '${timezone}') as date,
+      toDate(session_start, '${SqlString.escape(timezone)}') as date,
       count() as sessions
     FROM sessions
     WHERE

server/src/api/analytics/getUserSessions.ts

@@ -66,6 +66,9 @@ export async function getUserSessions(
   }
 
   const filterStatement = getFilterStatement(filters);
+  const timeStatement = getTimeStatement({
+    date: { startDate, endDate, timezone },
+  });
 
   const query = `
 SELECT
@@ -85,9 +88,7 @@ WHERE
     site_id = {siteId:Int32}
     AND user_id = {userId:String}
     ${filterStatement}
-    ${getTimeStatement({
-      date: { startDate, endDate, timezone },
-    })}
+    ${timeStatement}
 ORDER BY timestamp ASC
   `;
 

server/src/api/analytics/getUsers.ts

@@ -122,9 +122,7 @@ FROM events
 WHERE
     site_id = {siteId:Int32}
     ${filterStatement}
-    ${getTimeStatement({
-      date: { startDate, endDate, timezone },
-    })}
+    ${timeStatement}
   `;
 
   try {

server/src/api/analytics/query-validation.ts

@@ -148,7 +148,7 @@ const filterTypeSchema = z.enum([
 /**
  * Schema for filter parameter values
  */
-const filterParamSchema = z.enum([
+export const filterParamSchema = z.enum([
   "browser",
   "operating_system",
   "language",
@@ -185,7 +185,7 @@ const filterSchema = z.object({
  * @param params Raw input parameters
  * @returns Validated parameters
  */
-export function sanitizeTimeStatementParams(params: unknown) {
+export function validateTimeStatementParams(params: unknown) {
   return timeStatementParamsSchema.parse(params);
 }
 
@@ -195,7 +195,7 @@ export function sanitizeTimeStatementParams(params: unknown) {
  * @param bucket Raw bucket parameter
  * @returns Validated parameters and bucket
  */
-export function sanitizeTimeStatementFillParams(
+export function validateTimeStatementFillParams(
   params: unknown,
   bucket: unknown
 ) {
@@ -213,7 +213,7 @@ export function sanitizeTimeStatementFillParams(
  * @param filtersStr JSON string of filters
  * @returns Validated array of filter objects
  */
-export function sanitizeFilters(filtersStr: string) {
+export function validateFilters(filtersStr: string) {
   // First validate it's proper JSON
   let parsed: unknown;
   try {

server/src/api/analytics/utils.ts

@@ -1,9 +1,11 @@
 import { ResultSet } from "@clickhouse/client";
 import { Filter, FilterParameter, FilterType } from "./types.js";
 import {
-  sanitizeTimeStatementParams,
-  sanitizeFilters,
-} from "./sql-sanitziation.js";
+  validateTimeStatementParams,
+  validateFilters,
+  filterParamSchema,
+} from "./query-validation.js";
+import SqlString from "sqlstring";
 
 export function getTimeStatement({
   date,
@@ -18,7 +20,7 @@ export function getTimeStatement({
   pastMinutes?: number;
 }) {
   // Sanitize inputs with Zod
-  const sanitized = sanitizeTimeStatementParams({ date, pastMinutes });
+  const sanitized = validateTimeStatementParams({ date, pastMinutes });
 
   if (sanitized.date) {
     const { startDate, endDate, timezone, table } = sanitized.date;
@@ -28,21 +30,31 @@ export function getTimeStatement({
 
     const col = (table ?? "events") === "events" ? "timestamp" : "session_end";
 
+    // Use SqlString.escape for date and timezone values
     return `AND ${col} >= toTimeZone(
-      toStartOfDay(toDateTime('${startDate}', '${timezone}')),
+      toStartOfDay(toDateTime(${SqlString.escape(
+        startDate
+      )}, ${SqlString.escape(timezone)})),
       'UTC'
       )
       AND ${col} < if(
-        toDate('${endDate}') = toDate(now(), '${timezone}'),
+        toDate(${SqlString.escape(endDate)}) = toDate(now(), ${SqlString.escape(
+      timezone
+    )}),
         now(),
         toTimeZone(
-          toStartOfDay(toDateTime('${endDate}', '${timezone}')) + INTERVAL 1 DAY,
+          toStartOfDay(toDateTime(${SqlString.escape(
+            endDate
+          )}, ${SqlString.escape(timezone)})) + INTERVAL 1 DAY,
           'UTC'
         )
       )`;
   }
   if (sanitized.pastMinutes) {
-    return `AND timestamp > now() - interval '${sanitized.pastMinutes} minute'`;
+    // Use SqlString.escape for pastMinutes (it handles numbers)
+    return `AND timestamp > now() - interval ${SqlString.escape(
+      sanitized.pastMinutes
+    )} minute`;
   }
 }
 
@@ -86,7 +98,7 @@ export const geSqlParam = (parameter: FilterParameter) => {
   if (parameter === "dimensions") {
     return "concat(toString(screen_width), 'x', toString(screen_height))";
   }
-  return parameter;
+  return filterParamSchema.parse(parameter);
 };
 
 export function getFilterStatement(filters: string) {
@@ -95,7 +107,7 @@ export function getFilterStatement(filters: string) {
   }
 
   // Sanitize inputs with Zod
-  const filtersArray = sanitizeFilters(filters);
+  const filtersArray = validateFilters(filters);
 
   if (filtersArray.length === 0) {
     return "";
@@ -121,9 +133,9 @@ export function getFilterStatement(filters: string) {
                 FROM events 
                 GROUP BY session_id
               ) 
-              WHERE entry_pathname ${filterTypeToOperator(filter.type)} '${x}${
-              filter.value[0]
-            }${x}'
+              WHERE entry_pathname ${filterTypeToOperator(
+                filter.type
+              )} ${SqlString.escape(x + filter.value[0] + x)}
             )`;
           }
 
@@ -131,7 +143,7 @@ export function getFilterStatement(filters: string) {
             (value) =>
               `entry_pathname ${filterTypeToOperator(
                 filter.type
-              )} '${x}${value}${x}'`
+              )} ${SqlString.escape(x + value + x)}`
           );
 
           return `session_id IN (
@@ -158,9 +170,9 @@ export function getFilterStatement(filters: string) {
                 FROM events 
                 GROUP BY session_id
               ) 
-              WHERE exit_pathname ${filterTypeToOperator(filter.type)} '${x}${
-              filter.value[0]
-            }${x}'
+              WHERE exit_pathname ${filterTypeToOperator(
+                filter.type
+              )} ${SqlString.escape(x + filter.value[0] + x)}
             )`;
           }
 
@@ -168,7 +180,7 @@ export function getFilterStatement(filters: string) {
             (value) =>
               `exit_pathname ${filterTypeToOperator(
                 filter.type
-              )} '${x}${value}${x}'`
+              )} ${SqlString.escape(x + value + x)}`
           );
 
           return `session_id IN (
@@ -187,14 +199,14 @@ export function getFilterStatement(filters: string) {
         if (filter.value.length === 1) {
           return `${geSqlParam(filter.parameter)} ${filterTypeToOperator(
             filter.type
-          )} '${x}${filter.value[0]}${x}'`;
+          )} ${SqlString.escape(x + filter.value[0] + x)}`;
         }
 
         const valuesWithOperator = filter.value.map(
           (value) =>
             `${geSqlParam(filter.parameter)} ${filterTypeToOperator(
               filter.type
-            )} '${x}${value}${x}'`
+            )} ${SqlString.escape(x + value + x)}`
         );
 
         return `(${valuesWithOperator.join(" OR ")})`;