Search code examples
sqlsql-servert-sqlsql-server-2016

Why is a query under a IF statement that is false running?


I have a application that uses a lot of string interpolation for SQL queries. I know it is a SQL injection threat, this is something that the customer and us know about and is hopefully something we can focus on next big refactor. I say that to make sense of the {Root Container.property} things that come from a GUI.

I have this query

IF ({Root Container.UserSelectedProduct}=1)
    begin
        DECLARE @TestNumbers {Root Container.SQLProductType};
        INSERT INTO @TestNumbers SELECT * FROM {Root Container.DBTable};
        SELECT *
        FROM {Root Container.SQLProductFunction} (@TestNumbers)
        WHERE [ID] = {Root Container.Level};
    end
else
    Select 0

Before a user selects a product it looks like this

IF (0=1)     
  BEGIN
    DECLARE @TestNumbers myDataType;
    INSERT INTO @TestNumbers SELECT * FROM [MySchema].[TheWrongTable];     
    SELECT * FROM [dbo].[myfunction] (@TestNumbers)
    WHERE [ID] = 1;
  END
ELSE
  SELECT 0

Which is giving me the error:

Column name or number of supplied values does not match table definition.

I am aware why this error shows up, the table I am selecting from is not made for that data type.

However, why is it even attempting to run the first IF clause when I have IF (0=1) - how come this part is not just skipped and the SELECT 0 is only run? I would have thought that is how it was supposed to work, but I keep getting the error regarding column name/number not matching the table definition. When the user does select a Product and I get IF (1=1) and I have the appropriate table/function/datatype, it all works smoothly. I just don't know why it throws me an error prior when IF(1=0). Why does this happen/how can I get my intended behavior that everything inside my BEGIN\END under my first IF statement does not run unless the expression is true.


Solution

  • T-SQL is not interpreted. It must make sense regardless of what the runtime conditions are. It doesn't even do short-circuiting, in fact. Your code is invalid, and it doesn't matter that it's unreachable - T-SQL isn't going to ignore a piece of invalid code just because it could be eliminated, that's a thing that is a common source of bugs (e.g. in C++ where it's pretty common with templates).

    Just make sure you still get valid SQL for the case where no product is selected; use the wrong table (or a helper table) if you have to.