Two Dynamic-SQL Procs — One Injectable, One Correct

The most useful teaching artefact I found in this estate is not a clever algorithm or a subtle bug. It is a coincidence: two stored procedures, living in the same database, written to solve the same problem — return a page of report rows, sorted by a column the user picked — and they solve it in opposite ways. One concatenates user input straight into an EXEC string. The other uses a whitelist, QUOTENAME, and sp_executesql with typed parameters. The injectable one and the correct one, side by side, same schema, same purpose. You could not build a better before-and-after if you tried. This part reads both.

The shared problem: sortable, paged results

Both procedures back a grid: the user sees report rows, clicks a column header to sort, pages through results. That requires two pieces of information to reach SQL as variables — a sort column name and a direction — and one of them, the column name, is fundamentally an identifier, not a value. You cannot pass an ORDER BY column as a parameter; ORDER BY @col sorts by the constant string, not the column it names. This is the exact crevice where dynamic SQL creeps into otherwise-parameterised codebases, and it is worth understanding why the problem is genuinely hard before judging the wrong answer too harshly: T-SQL has no first-class way to parameterise an identifier. Something has to build a string.

The wrong answer, in full

The first procedure builds that string the direct way:

CREATE PROCEDURE dbo.GetMonthReport
    @SortCol   NVARCHAR(100),
    @SortDir   NVARCHAR(4),
    @StartDate NVARCHAR(20),
    @EmpId     NVARCHAR(20)
AS
BEGIN
    DECLARE @sql NVARCHAR(MAX);

    SET @sql = 'SELECT * FROM dbo.DynamicReportView
                WHERE EmployeeId = ' + @EmpId + '
                  AND LogDate >= ''' + @StartDate + '''
                ORDER BY ' + @SortCol + ' ' + @SortDir;

    EXEC(@sql);   -- --TODO  (the comment is real; the fix never came)
end

Every one of those + operators is a wound. @EmpId is spliced in unquoted — a classic numeric-context injection: pass 0 OR 1=1 and the WHERE evaporates, returning every employee's attendance. @StartDate is wrapped in quotes, so it takes the textbook escape — a value of 2020-01-01'; DROP TABLE dbo.Tolerance; -- closes the literal and appends a second statement. And @SortCol and @SortDir are concatenated as bare identifiers with no quoting at all, the widest hole of the three, because they land in a position where arbitrary SQL is expected to be syntactically valid.

The reach is not academic. DynamicReportView exposes attendance, names, managers and allowance data; EXEC runs under the application's SQL identity, which — because the estate barely uses roles or least privilege — can read and write far more than this view. A parameterised application layer in front does not save it: the moment a stored procedure concatenates its parameters, the injection just moves inside the database, past every guard the C# side put up. And the --TODO sitting inside the string is the saddest artefact in the estate — someone knew, wrote a note, and shipped it. A --TODO next to string concatenation in SQL is a CVE with a comment.

The right answer, in full

Now the second procedure. Same grid, same sort-column-and-direction problem, entirely different technique:

CREATE PROCEDURE dbo.ReportsQuery
    @SortCol   SYSNAME,
    @SortDir   NVARCHAR(4),
    @StartDate DATE,
    @EmpId     INT
AS
BEGIN
    DECLARE @orderCol SYSNAME =
        CASE @SortCol
            WHEN 'LogDate'      THEN 'LogDate'
            WHEN 'OfficeHours'  THEN 'OfficeHours'
            WHEN 'EmployeeName' THEN 'EmployeeName'
            ELSE 'LogDate'                       -- unknown input -> safe default
        END;

    DECLARE @dir NVARCHAR(4) =
        CASE WHEN @SortDir = 'DESC' THEN 'DESC' ELSE 'ASC' END;

    DECLARE @sql NVARCHAR(MAX) =
        N'SELECT * FROM dbo.DynamicReportView
          WHERE EmployeeId = @pEmpId AND LogDate >= @pStart
          ORDER BY ' + QUOTENAME(@orderCol) + N' ' + @dir;

    EXEC sp_executesql @sql,
        N'@pEmpId INT, @pStart DATE', @pEmpId = @EmpId, @pStart = @StartDate;
end

Read it against the first, line for line, because every defence is a direct answer to a specific wound:

• The values never touch the string. @EmpId and @StartDate are passed to sp_executesql as typed parameters — an INT and a DATE. There is no quoting to escape because there is no concatenation; the query text is fixed, and the values arrive through the parameter channel where injection is structurally impossible. The type declarations even reject the malformed inputs that fed the first proc's exploits.
• The identifier is whitelisted, then escaped. @SortCol cannot inject, because it is never used as SQL — it is used as a lookup key into a CASE that maps known names to known columns and everything else to a safe default. Only literal, hand-written column names ever reach the query. QUOTENAME then wraps the chosen name in brackets as belt-and-braces. The user picks from a menu; they do not get to write the menu.
• The direction is a binary choice, not a string. @SortDir collapses to exactly 'ASC' or 'DESC' before it can matter. The universe of possible values is two.

This is not exotic SQL. sp_executesql, QUOTENAME and a CASE whitelist are the standard, decades-old answer, in every guidance document Microsoft has published. The correct pattern was not hard, was not slow, and was not unknown — it was sitting in the same database.

The lesson is the coexistence, not the injection

SQL injection in 2020s code is barely worth an article on its own; every developer can recite the fix. What makes this pairing worth your time is that both procedures shipped, to production, in one database, maintained by one team. The knowledge to write ReportsQuery demonstrably existed — someone on the team held it and applied it. GetMonthReport was written anyway, or written earlier and never revisited, and nothing in the delivery pipeline objected. That is the finding: security is not a knowledge problem, it is a consistency problem. Teams don't lack the correct pattern; they lack the mechanism that makes the correct pattern the only one that reaches main.

What would have caught it? Not a training course — the training clearly took, once. A static analyser that flags EXEC(@variable) and passes sp_executesql would have caught it in seconds, and costs nothing to run in CI. A code-review checklist item — any dynamic SQL? show me the whitelist — would have caught it in the review that shipped it. A grep for EXEC( across the schema, run once, would have surfaced both procs and invited exactly the comparison this article makes. The estate had none of these, because — as part 17 will show — its pipeline ran dotnet test against projects with no tests and called it quality assurance. Good patterns don't propagate by being good. They propagate by being enforced.

Speaking of things the schema failed to enforce: the injectable proc could rewrite half the database partly because the database declines to protect itself — fourteen foreign keys across thirty-eight tables, none on the high-volume ones, and an index still named after the SSMS placeholder that generated it. Next, fourteen foreign keys and an index named Placeholder.