Add more input type validation to prevent injection (#49)

SQL injection was possible on most string types, although it's impact was limited due to the client connection being `readonly`. The changes add more robust input validation using a combination of `zod` and type casts.
pinax-network · Nov 3, 2023 · f755963 · f755963
1 parent ffbffbe
commit f755963
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 13 deletions.
diff --git a/src/queries.ts b/src/queries.ts
@@ -1,5 +1,5 @@
 import { DEFAULT_SORT_BY, config } from './config.js';
-import { parseBlockId, parseLimit, parseTimestamp } from './utils.js';
+import { parseBlockId, parseBlockNumber, parseChain, parseLimit, parseSortBy, parseTimestamp } from './utils.js';
 import { chains } from './fetch/openapi.js';
 
 export interface Block {
@@ -26,16 +26,16 @@ export function getBlock(searchParams: URLSearchParams) {
             ["less", "<"],
         ]
         for ( const [key, operator] of operators ) {
-            const block_number = searchParams.get(`${key}_by_block_number`);
+            const block_number = parseBlockNumber(searchParams.get(`${key}_by_block_number`));
             const timestamp = parseTimestamp(searchParams.get(`${key}_by_timestamp`));
             if (block_number) where.push(`block_number ${operator} ${block_number}`);
             if (timestamp) where.push(`toUnixTimestamp(timestamp) ${operator} ${timestamp}`);
         }
 
         // equals
-        const chain = searchParams.get("chain");
+        const chain = parseChain(searchParams.get("chain"));
         const block_id = parseBlockId(searchParams.get("block_id"));
-        const block_number = searchParams.get('block_number');
+        const block_number = parseBlockNumber(searchParams.get('block_number'));
         const timestamp = parseTimestamp(searchParams.get('timestamp'));
         if (chain) where.push(`chain == '${chain}'`);
         if (block_id) where.push(`block_id == '${block_id}'`);
@@ -47,8 +47,8 @@ export function getBlock(searchParams: URLSearchParams) {
 
         // Sort and Limit
         const limit = parseLimit(searchParams.get("limit"));
-        const sort_by = searchParams.get("sort_by");
-        query += ` ORDER BY block_number ${sort_by ?? DEFAULT_SORT_BY}`
+        const sort_by = parseSortBy(searchParams.get("sort_by"));
+        query += ` ORDER BY block_number ${sort_by}`
         query += ` LIMIT ${limit}`
 
         return query;

diff --git a/src/utils.spec.ts b/src/utils.spec.ts
@@ -1,22 +1,50 @@
 import { expect, test } from "bun:test";
-import { parseBlockId, parseLimit, parseTimestamp } from "./utils.js";
-import { DEFAULT_MAX_LIMIT } from "./config.js";
+import { parseBlockId, parseBlockNumber, parseChain, parseLimit, parseSortBy, parseTimestamp } from "./utils.js";
+import { DEFAULT_MAX_LIMIT, DEFAULT_SORT_BY } from "./config.js";
 
 test("parseBlockId", () => {
     expect(parseBlockId()).toBeUndefined();
     expect(parseBlockId(null)).toBeUndefined();
+    expect(parseBlockId("invalid")).toBeUndefined();
     expect(parseBlockId("00fef8cf2a2c73266f7c0b71fb5762f9a36419e51a7c05b0e82f9e3bacb859bc")).toBe("00fef8cf2a2c73266f7c0b71fb5762f9a36419e51a7c05b0e82f9e3bacb859bc");
     expect(parseBlockId("0x00fef8cf2a2c73266f7c0b71fb5762f9a36419e51a7c05b0e82f9e3bacb859bc")).toBe("00fef8cf2a2c73266f7c0b71fb5762f9a36419e51a7c05b0e82f9e3bacb859bc");
 });
 
+test("parseBlockNumber", () => {
+    expect(parseBlockNumber()).toBeUndefined();
+    expect(parseBlockNumber(null)).toBeUndefined();
+    expect(parseBlockNumber(-1)).toBeUndefined();
+    expect(parseLimit("invalid")).toBeNaN();
+    expect(parseBlockNumber("10")).toBe(10);
+    expect(parseBlockNumber(10)).toBe(10);
+});
+
+test("parseChain", () => {
+    expect(parseChain()).toBeUndefined();
+    expect(parseChain(null)).toBeUndefined();
+    expect(parseChain("' OR 1=1)--")).toBeUndefined();
+    expect(parseChain("test")).toBe("test");
+    expect(parseChain("test123")).toBe("test123");
+});
+
 test("parseLimit", () => {
     expect(parseLimit()).toBe(1);
     expect(parseLimit(null)).toBe(1);
+    expect(parseLimit(-1)).toBe(1);
+    expect(parseLimit("invalid")).toBeNaN();
     expect(parseLimit("10")).toBe(10);
     expect(parseLimit(10)).toBe(10);
     expect(parseLimit(999999)).toBe(DEFAULT_MAX_LIMIT);
 });
 
+test("parseSortBy", () => {
+    expect(parseSortBy()).toBe(DEFAULT_SORT_BY);
+    expect(parseSortBy(null)).toBe(DEFAULT_SORT_BY);
+    expect(parseSortBy("invalid")).toBe(DEFAULT_SORT_BY);
+    expect(parseSortBy("ASC")).toBe("ASC");
+    expect(parseSortBy("DESC")).toBe("DESC");
+});
+
 test("parseTimestamp", () => {
     const seconds = 1672531200;
     expect(parseTimestamp()).toBeUndefined();
@@ -37,5 +65,3 @@ test("parseTimestamp", () => {
     expect(() => parseTimestamp(10)).toThrow("Invalid timestamp");
     expect(() => parseTimestamp("10")).toThrow("Invalid timestamp");
 });
-
-
diff --git a/src/utils.ts b/src/utils.ts
@@ -1,4 +1,33 @@
-import { config } from "./config.js";
+import { z } from 'zod';
+import { DEFAULT_SORT_BY, config } from "./config.js";
+
+export function parseBlockId(block_id?: string|null) {
+    // Match against hexadecimal string (with or without '0x' prefix)
+    if (!z.string().regex(/^(0x)?[a-fA-F0-9]+$/).safeParse(block_id).success) {
+        return undefined;
+    }
+
+    return block_id ? block_id.replace("0x", "") : undefined;
+}
+
+export function parseBlockNumber(number?: string|null|number) {
+    let value = undefined;
+    if (number) {
+        if (typeof number === "string") value = parseInt(number);
+        if (typeof number === "number") value = number;
+    }
+    // Must be non-negative number
+    if ( value && value <= 0 ) value = undefined;
+    return value;
+}
+
+export function parseChain(chain?: string|null) {
+    if (!z.string().regex(/^[a-zA-Z0-9]+$/).safeParse(chain).success) {
+        return undefined;
+    }
+
+    return chain;
+}
 
 export function parseLimit(limit?: string|null|number) {
     let value = 1; // default 1
@@ -7,12 +36,17 @@ export function parseLimit(limit?: string|null|number) {
         if (typeof limit === "number") value = limit;
     }
     // limit must be between 1 and maxLimit
+    if ( value <= 0 ) value = 1;
     if ( value > config.maxLimit ) value = config.maxLimit;
     return value;
 }
 
-export function parseBlockId(block_id?: string|null) {
-    return block_id ? block_id.replace("0x", "") : undefined;
+export function parseSortBy(sort_by?: string|null) {
+    if (!z.enum(["ASC", "DESC"]).safeParse(sort_by).success) {
+        return DEFAULT_SORT_BY;
+    }
+
+    return sort_by;
 }
 
 export function parseTimestamp(timestamp?: string|null|number) {