Thread: SQLite, is it possible to put a string into an argument?

Sure, start with
const char *template = "INSERT INTO %s VALUES(value1,value2,%s)";

Then do
sprintf(buff,template,myTable,myKey);

But you need to make sure you have extremely good input string sanitisation (and validation).
Sending malformed SQL queries into a database is a well documented DoS attack - read up on SQL injection attacks before proceeding.

Post something that segfaults.

But to be honest, if you can't find a relatively simple segfault, how are you going to defend SQL injection attacks?

Using scanf() to read the user input is NOT the way to go.

Use fgets() to read a line of input.
Parse the line to look for a table name.
Make sure it matches EXACTLY one of the known table names. Don't blindly assume something vague like "string of isascii() chars" is a strong enough test.

Users complaining that they can't access table "foo" (if foo is a valid table name) is a lot easier to fix than "WTF happened to my database!?"

I am curious to know: why do you need the table name to be variable? After all, you only mentioned getting the user's name as input, so where does the table name come from? If you are going to use the user's name as the table name, then you're probably doing something wrong, e.g., you need to go back and think about normalisation.

If you really do need the table name to be variable, then you pretty much have to use sprintf after checking that the input is valid, but take care to quote the table name (with double quotes). The value to be inserted should be bound to a parameter within a prepared statement, not handled with sprintf.

Originally Posted by Salem

But to be honest, if you can't find a relatively simple segfault, how are you going to defend SQL injection attacks?

This is for a university project and the requirements state we only need to have a program with basic interaction with a database, we wont be expected to guard against injection attacks. Saying that it will still have good string sanitation and validation.

Originally Posted by laserlight

I am curious to know: why do you need the table name to be variable? After all, you only mentioned getting the user's name as input, so where does the table name come from? If you are going to use the user's name as the table name, then you're probably doing something wrong, e.g., you need to go back and think about normalisation.

If you really do need the table name to be variable, then you pretty much have to use sprintf after checking that the input is valid, but take care to quote the table name (with double quotes). The value to be inserted should be bound to a parameter within a prepared statement, not handled with sprintf.

Sorry I should not have put the table name in bold, this isn't for the user to change. The application is a basic address book, the only inputs they will be giving are for fields like FIRST_NAME etc.

This is what I'm trying to do more or less.

Code:

cont char *userstring = "test";

queries[ind++] = ("INSERT INTO table VALUES('%s','string',0)", userstring);
retval = sqlite3_exec(handle,queries[ind-1],0,0,0);

Error I get is: warning: assignment from incompatible pointer type

Thank you for your replies so far I appreciate it. Sorry if I haven't been clear I'm have trouble with C as I'm not really used to imperative programming.

Originally Posted by Kashinoda

Sorry I should not have put the table name in bold, this isn't for the user to change. The application is a basic address book, the only inputs they will be giving are for fields like FIRST_NAME etc.

Ah, that makes sense.

Originally Posted by Kashinoda

Error I get is: warning: assignment from incompatible pointer type

Unfortunately, because you did not show your declaration of queries, I cannot tell you exactly why this is so. However, I note that the expression ("INSERT INTO table VALUES('%s','string',0)", userstring) expression uses the comma operator, thus the left subexpression is evaluated (it is just a string, so there's no side effect), then the right subexpression is evaluated and becomes the value of the comma expression, thus that line has a net effect:

Code:

queries[ind++] = userstring;

Why do you need an array of queries anyway?

Originally Posted by Kashinoda

This is what I'm trying to do more or less.

Stop using sqlite3_exec except for SQL statements for which everything is hard coded. The appropriate way to do this is to use sqlite3_prepare_v2 to prepare your statement with a parameter, then use sqlite3_bind_text to bind your string to the parameter, and finally use sqlite3_step to execute the prepared statement.