How to prevent SQL injection in Select Insert?

Question

I have my Select Insert statement and I'm wondering if I should prevent my code from SQL injection. This code is using BULK Insert and TEMP TABLE. I have never use this before and I'm not familiar if I can/should use cfqueryparam in this case or there is something else that I can apply in this case? Here is my code:

<cfquery datasource="testDB" name="InsertBulk">
    IF OBJECT_ID('tempdb..##TEMP_TBL') IS NOT NULL DROP TABLE ##TEMP_TBL;

    CREATE TABLE ##TEMP_TBL (#cols#)

    BULK INSERT ##TEMP_TBL
    FROM 'D:\myFiles\myTXT.txt'
    WITH (
        FIELDTERMINATOR = '\t', 
        ROWTERMINATOR = '\n'
    )
</cfquery>

<cfquery datasource="testDB" name="InsertUpdate">
    INSERT INTO myRecords(
        FIRST_NAME,
        LAST_NAME,
        GENDER,        
        DOB
    )
    SELECT 
        CASE WHEN LEN(LTRIM(RTRIM(FIRST_NAME))) <> 0 OR FIRST_NAME <> 'NULL' THEN FIRST_NAME END,
        CASE WHEN LEN(LTRIM(RTRIM(LAST_NAME))) <> 0 OR LAST_NAME <> 'NULL' THEN LAST_NAME END,
        CASE WHEN LEN(LTRIM(RTRIM(GENDER))) <> 0 OR GENDER <> 'NULL' THEN GENDER END,
        CASE WHEN LEN(LTRIM(RTRIM(BIRTH_DATE))) <> 0 OR BIRTH_DATE <> 'NULL' THEN BIRTH_DATE END
    FROM ##TEMP_TBL AS TempInsert 
    WHERE NOT EXISTS (
        SELECT * 
        FROM myRecords AS Dups
        WHERE Dups.userID = TempInsert.user_ID
    )

    UPDATE Records
    SET 
        Records.FIRST_NAME = CASE WHEN LEN(LTRIM(RTRIM(Temp.FIRST_NAME))) <> 0 OR Temp.FIRST_NAME <> 'NULL' THEN Temp.FIRST_NAME END,
        Records.LAST_NAME = CASE WHEN LEN(LTRIM(RTRIM(Temp.LAST_NAME))) <> 0 OR Temp.LAST_NAME <> 'NULL' THEN Temp.LAST_NAME END,
        Records.GENDER = CASE WHEN LEN(LTRIM(RTRIM(Temp.GENDER))) <> 0 OR Temp.GENDER <> 'NULL' THEN Temp.GENDER END,
        Records.DOB = CASE WHEN LEN(LTRIM(RTRIM(Temp.BIRTH_DATE))) <> 0 OR Temp.BIRTH_DATE <> 'NULL' THEN Temp.BIRTH_DATE END,
    FROM myRecords AS Records
        INNER JOIN ##TEMP_TBL AS Temp
            ON Records.userID = Temp.user_ID
    WHERE Records.userID = Temp.user_ID
</cfquery>

I have approached my problem using bulk because I tried to avoid using cfloop and creating multiple INSERT/UPDATE statements.

Asking if a specific thing is "ok" is fine on Stack Overflow, but for more open ended "improvement" requests, codereview.stackexchange.com is better suited. With this in mind, I've removed the last few sentences from your question. — Matt
– Matt, Commented Nov 1, 2016 at 8:34
@Matt I haven't heard of those two that you listed above. Thanks for letting me know. — espresso_coffee
– espresso_coffee, Commented Nov 1, 2016 at 13:12

Community · Accepted Answer · 2017-05-23 10:27:33Z

1

No, you cannot use cfqueryparam here. However, with the possible exception of #cols# you do not need it for those statements.

CFQueryparam is designed to prevent literals (ie simple strings, dates, etcetera) from being executed as sql commands. The SELECT statement does not need to be protected because it does not contain any literals that could be interpreted as commands.

The one possible risk in the statements above is the #cols# variable, which seems to represent a list of column names. Object names (column names, table names, etcetera) must be interpreted as part of a command. Using cfqueryparam is designed to prevent that from happening. So it cannot be used to protect the #cols# variable. If that variable is client supplied, you must do any scrubbing yourself.

Keep in mind, there are still risks even after data is saved to the database. Even if you are using cfqueryparam, malicious values can still be inserted the database. CFQueryparam does not magically make an input value safe. It just prevents any malicious commands within the value from being executed (and only in the current statement). It will not stop an INSERT or UPDATE from saving the value(s) to the database, and posing a risk later on. For applications using any kind of dynamic SQL, stored values can still pose a risk through second order SQL injection:

Second order SQL injection occurs when submitted values contain malicious commands that are stored rather than executed immediately. In some cases, the application may correctly encode an SQL statement and store it as valid SQL. Then, another part of that application without controls to protect against SQL injection might execute that stored SQL statement. This attack requires more knowledge of how submitted values are later used....

So it is very important to always use cfqueryparam, and to avoid dynamic SQL that does not use bind variables. For example in CF, avoid any use of PreserveSingleQuotes, or in SQL Server stored procedures, avoid EXEC and use sp_executeSQL if needed.

edited May 23, 2017 at 10:27

CommunityBot

11 silver badge

answered Oct 25, 2016 at 18:19

Leigh

28.9k10 gold badges58 silver badges109 bronze badges

Sign up to request clarification or add additional context in comments.

9 Comments

Leigh Over a year ago

As an aside, any data scrubbing and filtering (ie trim, etcetera) should be done to the temp table, not in the UPDATE. Keep the UPDATE short and sweet.

espresso_coffee Over a year ago

is there any way to prevent Injection? Your last paragraph makes me little confused on what I'm missing in my code.

Leigh Over a year ago

See the updated answer. In short, always cfqueryparam and avoid (unparameterized) dynamic sql.

espresso_coffee Over a year ago

can you provide any example of how cfqueryparam can be applied to my variable #cols#? Do I have to loop and set cfparam individually or there is some more efficient way to do that? Thanks for the answer.

Leigh Over a year ago

Like I said, you cannot. Trying to use cfqueryparam on a column name will just cause an error. There is no really no shortcut. You have to validate each name yourself against a list of "good" columns. Though it raises the question, why does the create portion need to be dynamic?

|

Collectives™ on Stack Overflow

How to prevent SQL injection in Select Insert?

1 Answer 1

9 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

9 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related