How to reduce vulnerability to cyber attacks from injection?

48 Views Asked by At

I have very little knowledge about SQL injection, and there are probably other threats I am unaware of out there for stealing company data, how would I make this python code better in regards to security?

merge_query = """
MERGE INTO sql_table_name AS Target
USING (
    VALUES {}
) AS Source (transaction_year, month_num, month_name, price_nt)
ON Target.transaction_year = Source.transaction_year 
AND Target.month_num = Source.month_num
WHEN MATCHED AND (Target.month_name != Source.month_name OR Target.price_nt != Source.price_nt) THEN
    UPDATE SET Target.month_name = Source.month_name, Target.price_nt = Source.price_nt
WHEN NOT MATCHED THEN
    INSERT (transaction_year, month_num, month_name, price_nt) VALUES (Source.transaction_year, Source.month_num, Source.month_name, Source.price_nt);
""".format(','.join(['(?,?,?,?)' for _ in range(len(data))]))

params = [item for sublist in data for item in sublist]

try:
    obj_crsr.execute(merge_query, params)

except Exception as e:
    obj_crsr.rollback()
    print(e)
    print("Transaction rolled back")

else:
    obj_cnxn.commit()
    obj_crsr.close()
    obj_cnxn.close()

This python code is given data in the following format:

[(2023, M12,    December,   541.44),
(2023,  M11,    November,   486.64),
(2023,  M10,    October,    468.23),
(2023,  M09,    September,  478.80),
(2023,  M08,    August,     475.41)]

And then is converted to a list in the params variable. That data looks like this:

['2023', 'M12', 'December', '541.442', '2023', 'M11', 'November', '486.639', '2023', 'M10', 'October', '468.226', '2023', 'M09', 'September', '478.802', '2023', 'M08', 'August', '475.411']

This is the only method I have found to work when trying to merge data into an existing table using PYODBC. I have heard of parameterizing the query or naming sql variables, but I don't know how to adapt existing examples/solution on stack overflow to my specific use case. Any suggestions are appreciated.

1

There are 1 best solutions below

0
Charlieface On BEST ANSWER

There is nothing wrong with your existing code as far as injection is concerned.

Injection only happens when user data is injected directly into the query. Here, that is not happening. What is happening is that a dynamic list of ? parameter markers are being injected, in a well-defined way (once per row), and the actual values are passed as a dynamic list of parameters. There is absolutely no scope for injection here.

Be aware of a few points though:

  • Using such code does tend to focus minds on "oh just inject the data". Might be worth putting in a comment such as only paramaters are injected here.
  • If there are no rows at all to insert then you get invalid code ( VALUES ), so you should check for that.
  • Dynamic numbers of parameters mean you get a recompile for every possible variation of parameters. This is not as many as would be had if you actually injected the data, but still a concern.
    A Table-Valued Parameter or a bulk insert into a temp table are therefore both better options than any of this, but neither are supported by pyodbc. fast_executemany or to_sql might be options, or BULK INSERT from an external file.
  • The maximum number of parameters is 2100, you will hit performance issues well before that though.