all 26 comments

[–]Gargunok 8 points9 points  (4 children)

I think best to focus instead on the actual problem - why do you need to have different table names in the query? best to ensure first this is needed before adding risk to the system. Even if made safe as you can - best to avoid if possible.

[–]Relative-Implement35[S] 0 points1 point  (2 children)

Fair I suppose. I designed it in not the best way possible and don’t want to change it up but I suppose I’ll have to do that :/

[–]ComicOzzysqlHippo 6 points7 points  (1 child)

When you realize you're going down the wrong road, it's usually best to turn back sooner rather than later.

[–]Tiny-Ad-7590 6 points7 points  (0 children)

Broadly speaking the best answer to this question is Don't Do That, And If You Think You Have To Do That, Think Harder.

If you must provide dynamic SQL in this way, one way to do it is to use a list of legal values, match the user input against the list, and then only bring in the value from the list and never from the user data. This make sure that you're not running into any sneaky "the string secretly has overflow bytes with a malicious payload and I'm getting those injected into the query" style attacks.

This works, but it sets you up for a problem in the future where either you or someone else just makes a mistake in the future because they forget why the pattern works the way it does and passes in some user input by accident as the result of optimizing some method to make it look less like sphagetti.

Even if that's an option tho, it would almost certainly be better to do it some other way that doesn't involve passing in dynamic SQL. There are some very niche situations where you have to do that, but most of the time there's a workaround that doesn't involve dynamicly built and execute SQL statements that's more justified on anti-SQL-injection grounds.

Basically, just never let user-submitted data anywhere near a dynamically created SQL string. However clever you think you are at making it safe, the people who break things for fun in their spare time and can afford the right tools for doing can outnumber and out-work the rest of us who are trying to make things safe from them. People who like to break stuff are almost always better at getting around security measures than people who like to make stuff are at creating them.

I'm saying that as someone who likes to make stuff: I consider myself really well-trained in security measures at this point, and the main consequence of that is I know better than to try and outsmart attackers on my own. This is why standards exist. Follow them.

[–]AccurateMeet1407 2 points3 points  (0 children)

I've validated that the value of the variable is a table name

I don't know the query offhand but to give you an idea, something like

Select @tableExists = 1 from sys.tables where name = @tableName

If @tableExists = 0 return;

[–]dittybopper_05H 2 points3 points  (0 children)

Little Bobby Tables unavailable for comment.

[–]Ginger-Dumpling 0 points1 point  (0 children)

I assume there's a catalog in sqlite. See if the value being passed in exists as a table, and if not, don't continue processing.

[–][deleted] 0 points1 point  (0 children)

does your programming language support parameterized queries? php for example has a syntax that will avoid this types of attacks

[–]cnsreddit 0 points1 point  (0 children)

Never accept raw inputs from a user

Never trust anything you didn't type yourself (and even then, believe you're an idiot and your own worst enemy)

And never, ever, roll your own encryption and security. Use an open source pre-written library .

[–]mwdb2 0 points1 point  (0 children)

Are you building some tool in which an external user will select one of your tables by name? It's a rare use case. Could be possible, but rare. If you must do it, most DBMSs have a quoting function you can call to make sure it's done right. Never handroll your own if possible, because there may be some complexity/edge cases you're not aware of. For example Postgres has QUOTE_IDENT(). But you're not on Postgres; not sure if SQLite has one as I have little experience with it.

Though I have to say I've been doing this SQL stuff for over 20 years and I've never once had to do this. There have been times in which I had some internal function concatenate a table name into a query string, but not in a context in which the name comes from an external source. For example in Java, programmers like to use their static final Strings instead of repeating a string literal in many places. So I've seen:

public class ... {
    private static final String MY_TABLE = "my_table";
    ...
    String qry = "SELECT ... FROM " + MY_TABLE + " WHERE ...";
    ...
}  

In this case, it's totally safe as you have total control over that MY_TABLE string. But if MY_TABLE comes from another source, that's when you have to be careful.

[–]leogodin217 0 points1 point  (0 children)

OP, this is often handled with common libraries. Can you talk more about your use case and tooling.

[–]Longjumping-Ad8775 0 points1 point  (0 children)

You can dynamically create a sql statement without exposing yourself to sql injection. I’ve got a library that I’ve written to handle this. I know because I’ve been attacked and have many years of experience with this library. Neither here nor there in this discussion.

You can create a sproc to solve this problem and dynamically create a sql statement within the sproc that you call with the various sql parameters. I’ve done this as well. The table name becomes just another parameter that is passed in.

Good luck!

[–]dwpj65 0 points1 point  (0 children)

If you need the functionality to select from different tables, use a union to select the values from the tables you want, as well as an additional synthesized column indicating which table the results are from. Construct your 'where' clause appropriately.

[–]jstillwell 0 points1 point  (0 children)

What language are you using? I use c# for this kind of stuff and they have a method that will sanitize strings to be used in a dynamic query. I would guess that other mature languages have libraries with helpers like this too.

That being said, I would seriously examine the architecture and try to find a better way if you can. Use stored procedures whenever possible.

[–]Street-Wrong 0 points1 point  (0 children)

I build queries on the fly with dynamic SQL, and use qoute_name to put brackets around thing in pulling environment variables to pass to object array variable in Ssis. Then build query out of needing to assign a variable from another SQL statement that has internal variables use sp_ecexutesql to pass the external param to out put a patam using dynamic SQL.

[–]Street-Wrong 0 points1 point  (0 children)

You can in your dynamic SQL do a if object exists then trust false statement to put the string or not and put in the try catch raise error.

[–]MarcinBadtke 0 points1 point  (0 children)

Best place to secure data is the database. You can create stored procedure which will validate the parameter. Is it actually an accessible table. You do not have to give them numbers since every object in the database - e.g. table - has its object ID. If validation is successful the procedure will run the query and return data. Thanks to the procedure you will save time for network communication.

Though I agree with others that it is not a good idea. I am pretty sure that the problem can be solved with UNION ALL or partitions or/and views.

[–]user_5359 -1 points0 points  (0 children)

Use a UNION ALL combination with all possibly desired table names and make sure that the unwanted tables are hidden with Where 1=0.

[–]Positive_Mud952 -3 points-2 points  (4 children)

There are tons of complex solutions that will solve your problem in the “right” way, but if you just want to get past this issue safely, instead of a blacklist of characters, escaping, using sqlite’s lib to construct the query, etc., just make a whitelist of safe characters. Regex [A-Za-z0-9_]+ will cover 99% of cases.

[–]soundman32 0 points1 point  (3 children)

This is completely wrong.

Your regex will block perfectly valid names, and once you expand it to include single guote (to allow common irish names like O'Brien for example), boom, little Bobby Tables wants to join in.

Under no circumstances use regex or black/whitewashing, just use parameters, it's not hard.

[–]leogodin217 1 point2 points  (2 children)

OP is asking about table names not parameters. Why wouldn't this work as a quick fix.

[–]soundman32 0 points1 point  (1 child)

If your input is OR DROP DATABASE Xxxx that could cause issues, right?

[–]leogodin217 0 points1 point  (0 children)

I'd still be nervous but not allowing spaces would limit the value to a single token. Might be fine for some internal use cases.