SQL sanitizing in code with no user input

Question

In my company's codebase, we hardcode sql queries without using an ORM.

Here's an example of a query we would run:

UPDATE client SET status="active" WHERE client_id=123

Since the query is hardcoded and the parameters are passed in, the code would look like this (Python):

'UPDATE client SET status="{}" WHERE client_id={}'.format('active', 123)

The only place that the parameters are coming from is the code where they are set. There is no case where external users can pass in arguments.

The codebase is not large but is quite sprawling and interrelated, so sanitizing the SQL would require a legitimate reason for the time spent.

Therefore, is there a reason to worry about sanitizing the SQL?

Answer 1

If there is never any user input at all, or the program is only used internally, the importance of sanitizing should be reduced.

But there are still a few possible reasons to always do so

1. If all queries are sanitized there is no risk for some developer to copy paste code to some place where it does matter.
2. It is possible that future changes may take input from some user. If this is done it will be much more expensive to go through all the code base and fix all the queries. Or worse, this is forgotten about.
3. Parametrized queries should only be marginally more complex to write, and may have other advantages with regards to performance and/or type safety. With some languages/libraries/frameworks it may be more complex to write a non-parametrized query, and intentionally so.
4. Figuring out if a query may take user input or not can be costly. It is probably easier to just use the safe version everywhere.
5. There is less chance of formatting issues, the format of many data types depend on the localization, and there is no guarantee that this will match the localization of the database.
6. Parametrized queries might be easier to read and maintain, but this might depend on programming language etc. But it will almost certainly be easier to read by someone used to and expecting all queries to be parametrized.

Answer 2

Parameterised queries should be your standard approach to all SQL. If you are trying to find reasons why you don't have to use them, then you are doing yourself a disservice.

In your example you have no parameters at all to pass, the very fact that you have written the sql as a formatted string rather than just putting the parameters in the string suggested that you do pass these in from somewhere and you should be using a parameterised query.

Hackers are sneaky, they don't need a text box to type 'bobby drop tables' in and have it passed right to the database, these tricks can be added to files, other databases, urls etc etc. No parameter is safe.

Answer 3

By using parameterized queries your SQL server doesn't have to recalculate the query execution plan each time you use a query.

This can improve performance for queries ran often.

Answer 4

I just want to point out that your example is actually extremely, extremely nasty, because somebody not familiar or not careful enough would glance at these parentheses and assume that you're using parametrized queries and that the input is already safe.

Your example:

'UPDATE client SET status="{}" WHERE client_id={}'.format('active', 123)

Somebody comes in 5 years later with a task to accept the status from an user input, sees something like this and just changes it to

'UPDATE client SET status="{}" WHERE client_id={}'.format(userInput['status'], 123)

and boom, you've got yourself a fully-fledged SQL injection.

If you're insistent of doing things your way, at least make it obvious that something weird is going on.

'UPDATE client SET status="' + 'active' + '" WHERE client_id=' + '123'

Or even better, just put the values directly so it's known that they're constant.

'UPDATE client SET status="active" WHERE client_id=123

Or, the best way, just use the damn parameters instead of wasting your time thinking about this stuff.

cursor.execute('UPDATE client SET status=? WHERE client_id=?', ('active', 123))

Humans fail all the time. Just try to make it a bit harder for them.

Answer 5

I'm going to contradict some of the other answers and say there's little reason to parametrize a query where all the values are hard-coded.

However, you generally shouldn't be hard-coding so much. In your query, 123 is a magic number, and these should generally be avoided. There should be a named variable (or constant, in languages that support them) that describes the meaning of the number.

And once you move that value into a variable, then you should use a query parameter to substitute it. While there may not be a security purpose for this, since the number isn't supplied by the user, it's still a good practice for the reasons given in the other answers.

client_foo = 123
cursor.execute('UPDATE client SET status="active" WHERE client_id=?', (client_foo,))

The meaning of active is self-evident, so hard-coding it is less of a problem. However, if the script has multiple queries that set different statuses, you may want to create a generic SQL query or procedure that can be used to set different statuses, and this would be best done with another parameter.

update_status = 'UPDATE client SET status=? WHERE client_id=?'
...
cursor.execute(update_status, ("active", client_foo))
cursor.execute(update_status, ("completed", client_bar))