BUG (P1/Security): @WhatIfOutputTable SQL injection defense is inert on CREATE path and incomplete on INSERT path

[nanoDBA]

---

Summary

The @WhatIfOutputTable parameter accepts a user-supplied table name used to dynamically build and execute a CREATE TABLE and subsequent INSERT statements. The procedure attempts to sanitize this value via a blocklist approach, but the defense is broken in two separate ways:

1. CREATE TABLE path: Blocklist validation fires after EXECUTE(@whatif_create_sql) at ~line 1817. The table is already created before the check runs. The blocklist is inert for this path.
2. INSERT path: The blocklist at ~lines 2000–2022 is incomplete. The following bypass vectors are not covered:
   • EXEC( — stacked execution via a nested call
   • CHAR() — encoding bypass (e.g., CHAR(59) for semicolon)
   • OPENROWSET — out-of-band data exfiltration
   • 0x hex literals — binary encoding bypass

---

Impact

A crafted @WhatIfOutputTable value can execute arbitrary T-SQL with the permissions of the procedure caller. In environments where sp_StatUpdate runs under a high-privilege service account (e.g., sysadmin or db_owner), this is a complete privilege escalation vector.

Example of a bypass via the INSERT path:

-- Hypothetical crafted value (hex encoding bypasses the CHAR() check omission):
@WhatIfOutputTable = N'dbo.t; INSERT INTO audit_log EXEC xp_cmdshell(0x...)'

---

Root Cause

-- CURRENT (v2.3) — CREATE path (~line 1817)
SET @whatif_create_sql = N'CREATE TABLE ' + @WhatIfOutputTable + N' (...)';
EXECUTE(@whatif_create_sql);    -- ← executed BEFORE blocklist check
-- blocklist check runs here — too late

-- CURRENT (v2.3) — INSERT path blocklist (~lines 2000–2022)
IF @WhatIfOutputTable LIKE '%DROP%'
OR @WhatIfOutputTable LIKE '%DELETE%'
OR @WhatIfOutputTable LIKE '%;%'
-- ... (missing EXEC(, CHAR(), OPENROWSET, 0x)

Blocklist approaches for SQL injection are fundamentally fragile. Any missed bypass vector (and there will always be missed vectors) creates a vulnerability.

---

Fix

Replace the blocklist on both paths with structural validation using PARSENAME + QUOTENAME. This makes injection structurally impossible regardless of input content.

-- PROPOSED FIX — validate and sanitize before any EXECUTE

DECLARE @safe_schema  sysname;
DECLARE @safe_table   sysname;
DECLARE @safe_fqn     nvarchar(512);

-- PARSENAME handles 1-part and 2-part names; returns NULL on invalid input
SET @safe_table  = PARSENAME(@WhatIfOutputTable, 1);
SET @safe_schema = ISNULL(PARSENAME(@WhatIfOutputTable, 2), N'dbo');

-- NULL means PARSENAME rejected it (too many parts, embedded quotes, etc.)
IF @safe_table IS NULL
BEGIN
    RAISERROR(
        '@WhatIfOutputTable value is not a valid table name. Use schema.table or table format.',
        16, 1
    );
    RETURN;
END

-- QUOTENAME escapes brackets; now safe to concatenate
SET @safe_fqn = QUOTENAME(@safe_schema) + N'.' + QUOTENAME(@safe_table);

-- Use @safe_fqn in all subsequent dynamic SQL
SET @whatif_create_sql = N'CREATE TABLE ' + @safe_fqn + N' (...)';
EXECUTE(@whatif_create_sql);

This pattern:

• Rejects any value that is not a valid 1- or 2-part SQL identifier
• Escapes bracket characters via QUOTENAME, preventing bracket injection
• Eliminates all encoding bypass vectors (they fail PARSENAME before reaching EXECUTE)
• Applies consistently to both CREATE TABLE and INSERT paths from a single validation point

---

Breaking Change Note

This change is technically breaking for any caller passing an @WhatIfOutputTable value that:

• Contains more than 2 name parts (e.g., server.db.schema.table — rejected by PARSENAME)
• Relies on unquoted reserved words as identifiers

In practice, no legitimate use case requires injection-vector syntax. This should be documented in CHANGELOG as a security hardening change.

---

Testing Requirements

• Test with valid 1-part name: mytable → accepted
• Test with valid 2-part name: dbo.mytable → accepted
• Test with 3-part name: mydb.dbo.mytable → rejected (PARSENAME returns NULL for part > 2 in this context)
• Test with semicolon injection: dbo.t; DROP TABLE x → rejected
• Test with CHAR() bypass: dbo.t' + CHAR(59) + 'DROP TABLE x → rejected
• Test with OPENROWSET in name → rejected
• Test with 0x hex literal → rejected
• Confirm CREATE TABLE fires only AFTER validation passes

---

References

• PARSENAME for injection-safe name handling
• QUOTENAME docs: sys.fn_quotename / QUOTENAME
• v2.3 blocklist location: ~lines 1817, 2000–2022