Fix denylist bypass via quoted commands (#472)

* Fix denylist bypass via quoted commands

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Fix ReDoS vulnerability in parseCommandArgs regex

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Fix lint: use double quotes in test file

Signed-off-by: ravjotb <ravjot.brar@improving.com>

* Replace regex parser with iterative parser to fix ReDoS

Signed-off-by: ravjotb <ravjot.brar@improving.com>

---------

Signed-off-by: ravjotb <ravjot.brar@improving.com>

Author: ravjotbrar

apps/frontend/src/components/send-command/SendCommand.tsx

@@ -4,7 +4,7 @@ import { useSelector } from "react-redux"
 import { useParams } from "react-router"
 import { toast } from "sonner"
 import { truncateText } from "@common/src/truncate-text"
-import { findBlockedCommand, findConfirmCommand } from "@common/src/command-restrictions"
+import { findBlockedCommand, findConfirmCommand, parseCommandArgs } from "@common/src/command-restrictions"
 import type { JSONObject } from "@common/src/json-utils.ts"
 import { matchCommands, type MatchResult, type ValkeyCommand } from "@/components/send-command/valkey-command-matching"
 import { CommandAutocomplete } from "@/components/send-command/CommandAutocomplete"
@@ -77,14 +77,15 @@ export function SendCommand() {
 
   const onSubmit = (command?: string) => {
     const cmd = command || text
+    const parsedArgs = parseCommandArgs(cmd)
 
-    const blocked = findBlockedCommand(cmd)
+    const blocked = findBlockedCommand(parsedArgs)
     if (blocked) {
       toast.error(`Command blocked: ${blocked.reason}`)
       return
     }
 
-    const confirm = findConfirmCommand(cmd)
+    const confirm = findConfirmCommand(parsedArgs)
     if (confirm) {
       setPendingConfirm({ command: cmd, reason: confirm.reason })
       return

apps/server/src/actions/command.ts

@@ -1,4 +1,4 @@
-import { VALKEY, findBlockedCommand } from "valkey-common"
+import { VALKEY, findBlockedCommand, parseCommandArgs } from "valkey-common"
 import { sendValkeyRunCommand } from "../send-command"
 import { type Deps, withDeps } from "./utils"
 
@@ -11,7 +11,7 @@ export const sendRequested = withDeps<Deps, void>(
   async ({ ws, clients, connectionId, action }) => {
     const payload = action.payload as CommandAction
 
-    const blocked = findBlockedCommand(payload.command)
+    const blocked = findBlockedCommand(parseCommandArgs(payload.command))
     if (blocked) {
       ws.send(
         JSON.stringify({

apps/server/src/send-command.ts

@@ -1,19 +1,13 @@
 import { GlideClient, GlideClusterClient, ConnectionError, ClosingError, TimeoutError } from "@valkey/valkey-glide"
 import WebSocket from "ws"
-import { VALKEY } from "valkey-common"
+import { VALKEY, parseCommandArgs } from "valkey-common"
 import { parseResponse } from "./utils"
 
 export const isRequestError = (x: unknown): x is Error | string =>
   x instanceof Error ||
   (typeof x === "string" && x.startsWith("ResponseError:"))
 
-/**
- * Parses a command string into arguments, respecting quoted strings and escaped quotes.
- * Matches valkey-cli behavior: 'GET "my key"' → ['GET', 'my key']
- */
-export const parseCommandArgs = (command: string): string[] =>
-  command.trim().match(/(?:[^\s"']+|"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*')+/g)
-    ?.map((arg) => arg.replace(/^["']|["']$/g, "").replace(/\\(["'])/g, "$1")) ?? []
+export { parseCommandArgs }
 
 export async function sendValkeyRunCommand(
   client: GlideClient | GlideClusterClient,

common/src/__tests__/command-restrictions.test.ts

@@ -0,0 +1,62 @@
+import { describe, it } from "node:test"
+import assert from "node:assert"
+import { findBlockedCommand, findConfirmCommand, parseCommandArgs } from "../command-restrictions"
+
+describe("command restrictions", () => {
+  describe("findBlockedCommand", () => {
+    it("blocks FLUSHALL", () => {
+      assert.ok(findBlockedCommand(parseCommandArgs("FLUSHALL")))
+    })
+
+    it("blocks FLUSHALL with arguments", () => {
+      assert.ok(findBlockedCommand(parseCommandArgs("FLUSHALL ASYNC")))
+    })
+
+    it("blocks quoted FLUSHALL (bypass fix)", () => {
+      assert.ok(findBlockedCommand(parseCommandArgs("\"FLUSHALL\"")))
+    })
+
+    it("blocks single-quoted FLUSHALL (bypass fix)", () => {
+      assert.ok(findBlockedCommand(parseCommandArgs("'FLUSHALL'")))
+    })
+
+    it("blocks SHUTDOWN regardless of quoting", () => {
+      assert.ok(findBlockedCommand(parseCommandArgs("\"SHUTDOWN\"")))
+    })
+
+    it("blocks DEBUG regardless of quoting", () => {
+      assert.ok(findBlockedCommand(parseCommandArgs("\"DEBUG\" SLEEP 1")))
+    })
+
+    it("does not block normal commands", () => {
+      assert.strictEqual(findBlockedCommand(parseCommandArgs("GET mykey")), undefined)
+    })
+
+    it("is case-insensitive", () => {
+      assert.ok(findBlockedCommand(parseCommandArgs("flushall")))
+      assert.ok(findBlockedCommand(parseCommandArgs("\"flushdb\"")))
+    })
+  })
+
+  describe("findConfirmCommand", () => {
+    it("requires confirmation for KEYS", () => {
+      assert.ok(findConfirmCommand(parseCommandArgs("KEYS *")))
+    })
+
+    it("requires confirmation for quoted KEYS (bypass fix)", () => {
+      assert.ok(findConfirmCommand(parseCommandArgs("\"KEYS\" *")))
+    })
+
+    it("requires confirmation for CLUSTER RESET", () => {
+      assert.ok(findConfirmCommand(parseCommandArgs("CLUSTER RESET")))
+    })
+
+    it("requires confirmation for quoted CLUSTER RESET", () => {
+      assert.ok(findConfirmCommand(parseCommandArgs("\"CLUSTER\" \"RESET\"")))
+    })
+
+    it("does not confirm normal commands", () => {
+      assert.strictEqual(findConfirmCommand(parseCommandArgs("GET mykey")), undefined)
+    })
+  })
+})

common/src/command-restrictions.ts

@@ -3,6 +3,57 @@ export type CommandRestriction = {
   reason: string
 }
 
+/**
+ * Parses a command string into arguments, respecting quoted strings and escaped quotes.
+ * Matches valkey-cli behavior: 'GET "my key"' → ['GET', 'my key']
+ */
+export const parseCommandArgs = (command: string): string[] => {
+  const args: string[] = []
+  const input = command.trim()
+  let i = 0
+
+  while (i < input.length) {
+    // Skip whitespace
+    while (i < input.length && /\s/.test(input[i])) i++
+    if (i >= input.length) break
+
+    let arg = ""
+    if (input[i] === "\"") {
+      // Double-quoted string
+      i++ // skip opening quote
+      while (i < input.length && input[i] !== "\"") {
+        if (input[i] === "\\" && i + 1 < input.length) {
+          i++ // skip backslash
+        }
+        arg += input[i]
+        i++
+      }
+      i++ // skip closing quote
+    } else if (input[i] === "'") {
+      // Single-quoted string
+      i++ // skip opening quote
+      while (i < input.length && input[i] !== "'") {
+        if (input[i] === "\\" && i + 1 < input.length) {
+          i++ // skip backslash
+        }
+        arg += input[i]
+        i++
+      }
+      i++ // skip closing quote
+    } else {
+      // Unquoted token
+      while (i < input.length && !/[\s"']/.test(input[i])) {
+        arg += input[i]
+        i++
+      }
+    }
+
+    args.push(arg)
+  }
+
+  return args
+}
+
 // these commands are blocked and cannot be executed because they can cause server problems
 export const BLOCKED_COMMANDS: CommandRestriction[] = [
   { pattern: ["SHUTDOWN"], reason: "SHUTDOWN stops the server and cannot be undone remotely." },
@@ -21,18 +72,18 @@ export const CONFIRM_COMMANDS: CommandRestriction[] = [
   { pattern: ["CLUSTER", "RESET"], reason: "CLUSTER RESET resets the cluster state and may cause data loss." },
 ]
 
-export function matchesRestriction(command: string, restriction: CommandRestriction): boolean {
-  const parts = command.trim().toUpperCase().split(/\s+/)
+export function matchesRestriction(parsedArgs: string[], restriction: CommandRestriction): boolean {
+  const parts = parsedArgs.map((p) => p.toUpperCase())
   return (
     restriction.pattern.length <= parts.length &&
     restriction.pattern.every((token, i) => parts[i] === token)
   )
 }
 
-export function findBlockedCommand(command: string): CommandRestriction | undefined {
-  return BLOCKED_COMMANDS.find((r) => matchesRestriction(command, r))
+export function findBlockedCommand(parsedArgs: string[]): CommandRestriction | undefined {
+  return BLOCKED_COMMANDS.find((r) => matchesRestriction(parsedArgs, r))
 }
 
-export function findConfirmCommand(command: string): CommandRestriction | undefined {
-  return CONFIRM_COMMANDS.find((r) => matchesRestriction(command, r))
+export function findConfirmCommand(parsedArgs: string[]): CommandRestriction | undefined {
+  return CONFIRM_COMMANDS.find((r) => matchesRestriction(parsedArgs, r))
 }