Re: [sqlite] Bug in sqlite3_step

2012-07-04 Thread Nico Williams 
On Wed, Jul 4, 2012 at 8:06 AM, Igor Tandetnik  wrote:
> Nico Williams  wrote:
>> SQLite3 also needs to know the identifiers of schema elements at
>> statement prep time.  It might be nice to have a variant of
>> sqlite3_prepare_v2() that takes a varargs list of parameters which
>> must be identifiers, and then have a syntax for referring to
>> identifier parameters as opposed to value parameters.
>
> That doen't make much sense. The query plan for "select * from table1 where 
> col1=?" may be completely different from one for "select * from table2 where 
> col2=?". What exactly do you expect sqlite3_prepare_v2 to prepare, if table 
> and column could vary afterwards? Also, what are sqlite3_column_count, 
> sqlite3_column_decltype et al supposed to return?

Precisely, which is why any identifiers (table names, column names)
have to be known at statement prep time.  But using parametrized
queries adds some safety, so it makes sense to me to have two types of
parameters: those which must be bound at statement prep time, and
those that can be bound later.
___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-04 Thread Jay A. Kreibich 
On Wed, Jul 04, 2012 at 08:29:33AM -0500, Jay A. Kreibich scratched on the wall:
> On Wed, Jul 04, 2012 at 01:09:01AM -0500, Nico Williams scratched on the wall:
> 
> > But this would
> > just be a glorified (if safer) variant of sqlite3_mprintf() -- for
> > apps that allow users to manipulate the schema this could actually be
> > a good improvement.
> 
>   The sqlite3_*printf() family supports the %w option specifically
>   for the safe formatting identifiers.

   ...and someone just pointed out that %w isn't documented on the
   SQLite site: http://sqlite.org/c3ref/mprintf.html

   Sorry about that.  I'm not sure if that's an oversight in the docs,
   or if it is an undocumented feature.



   See "Using SQLite" ,
   Apdx G, p474-475 for more info.  It seems these pages are included
   in Amazon's "Look Inside" feature (at least for me):
   .


   Or see the SQLite source.

-j

-- 
Jay A. Kreibich < J A Y  @  K R E I B I.C H >

"Intelligence is like underwear: it is important that you have it,
 but showing it to the wrong people has the tendency to make them
 feel uncomfortable." -- Angela Johnson
___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-04 Thread Jay A. Kreibich 
On Wed, Jul 04, 2012 at 09:06:16AM -0400, Igor Tandetnik scratched on the wall:
> Nico Williams  wrote:
> > SQLite3 also needs to know the identifiers of schema elements at
> > statement prep time.  It might be nice to have a variant of
> > sqlite3_prepare_v2() that takes a varargs list of parameters which
> > must be identifiers, and then have a syntax for referring to
> > identifier parameters as opposed to value parameters.
> 
> That doen't make much sense.

  Yes it does.  The identifiers would be baked into the statement
  before it is prepared.  He's not trying to alter the identifiers after
  the statement is prepared, he's trying to prevent SQL-injection
  attacks while the statement is prepared.

  The #1 rule to prevent SQL-injection attacks is, "Never let user-generated
  strings pass through the SQL parser".  Statement re-use is nice, but
  the real value in using SQL statements and parameters is that you
  avoid passing data values through the SQL parser, making SQL-injection
  attacks impossible.  99% of SQL-injection attacks could be avoided if
  the programmer had only used SQL parameters rather than string
  concatenation.  It's also why sqlite3_exec() should really include a
  vararg option, so that one could pass in values outside the SQL
  string itself.

  Unfortunately, you can't use parameters for everything.  If you get
  in a situation where you need to use a user-defined table name,
  parameters won't help you.

  The (distant) #2 rule to prevent SQL-injection attacks is, "Sanitize
  user-generated strings before they pass through the SQL parser."  The
  issue with that is that too many programmers think themselves clever
  and smart, so they write their own sanitizer, and they do so poorly.
  Hence the popularity of "tried, true, and tested" string sanitizers
  being built into SQL libraries.  That's what Nico is looking for.

  Thankfully, SQLite includes this functionality, just not in the
  _prepare() functions.  SQLite supports several extensions to the
  standard printf() syntax in the sqlite3_*printf() family of
  functions.  Both %q and %Q can be used for values, while %w can be
  used for identifiers.  The sqlite3_*printf() functions will properly
  quote and sanitize any value in the generated string.

  There is also a %z and %p, but they're not really important for this
  discussion.

   -j

-- 
Jay A. Kreibich < J A Y  @  K R E I B I.C H >

"Intelligence is like underwear: it is important that you have it,
 but showing it to the wrong people has the tendency to make them
 feel uncomfortable." -- Angela Johnson
___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-04 Thread Jay A. Kreibich 
On Wed, Jul 04, 2012 at 01:09:01AM -0500, Nico Williams scratched on the wall:

> But this would
> just be a glorified (if safer) variant of sqlite3_mprintf() -- for
> apps that allow users to manipulate the schema this could actually be
> a good improvement.

  The sqlite3_*printf() family supports the %w option specifically
  for the safe formatting identifiers.  Like the %q option, you need to
  include the quotes in your string literal.  So your modified prepare
  would look something like this:

  sql_str = sqlite3_mprintf( "DROP TABLE \"%w\"", table_name );
  sqlite3_prepare_v2( db, sql_str, -1, , NULL );
  sqlite3_free( sql_str );

   -j

-- 
Jay A. Kreibich < J A Y  @  K R E I B I.C H >

"Intelligence is like underwear: it is important that you have it,
 but showing it to the wrong people has the tendency to make them
 feel uncomfortable." -- Angela Johnson
___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-04 Thread Igor Tandetnik 
Nico Williams  wrote:
> SQLite3 also needs to know the identifiers of schema elements at
> statement prep time.  It might be nice to have a variant of
> sqlite3_prepare_v2() that takes a varargs list of parameters which
> must be identifiers, and then have a syntax for referring to
> identifier parameters as opposed to value parameters.

That doen't make much sense. The query plan for "select * from table1 where 
col1=?" may be completely different from one for "select * from table2 where 
col2=?". What exactly do you expect sqlite3_prepare_v2 to prepare, if table and 
column could vary afterwards? Also, what are sqlite3_column_count, 
sqlite3_column_decltype et al supposed to return?
-- 
Igor Tandetnik

___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-04 Thread Nico Williams 
SQLite3 also needs to know the identifiers of schema elements at
statement prep time.  It might be nice to have a variant of
sqlite3_prepare_v2() that takes a varargs list of parameters which
must be identifiers, and then have a syntax for referring to
identifier parameters as opposed to value parameters.  But this would
just be a glorified (if safer) variant of sqlite3_mprintf() -- for
apps that allow users to manipulate the schema this could actually be
a good improvement.

Nico
--
___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-03 Thread Dan Kennedy 

On 07/04/2012 03:30 AM, Jay A. Kreibich wrote:

On Tue, Jul 03, 2012 at 04:26:42PM -0400, Richard Hipp scratched on the wall:


Actually, you can bind on a DDL statement, but bindings are only valid for
the lifetime of the statement itself, not for the whole lifetime of the
object created by the CREATE statement.


   Is that a side-effect of the fact that CREATE statements are copied
   into the sqlite_master table as literals, and not re-written?  (Is
   that even true?)


Pretty much. SQLite messes around with the start of CREATE TABLE
statements to remove the "TEMP" keyword or fully qualified database
name if one is present, but otherwise just copies the users input
into the sqlite_master table. i.e. if you do:

  CREATE TABLE main.x1(a, b);

SQLite rewrites "CREATE TABLE main." as "CREATE TABLE ". The
rest of the CREATE TABLE is stored in sqlite_master as is.


___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-03 Thread Igor Tandetnik 
Robert Myers  wrote:
> DROP TABLE ? would've been useful for me.

Parameters can only appear where literals would be allowed by the syntax. A 
table name is not a literal.
-- 
Igor Tandetnik

___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-03 Thread Robert Myers 

On 7/3/2012 3:26 PM, Richard Hipp wrote:


Igor is correct.

Actually, you can bind on a DDL statement, but bindings are only valid for
the lifetime of the statement itself, not for the whole lifetime of the
object created by the CREATE statement.  So doing such bindings are
pointless.  And you can't really bind to a DROP statement, because there
are no expressions in a DROP statement to bind to.  But these are all
details.  Igor's explanation is simple and to the point.



DROP TABLE ? would've been useful for me. As it is, I just generate the 
statement on the fly. That's something that gives me the willies, even 
though I'm fairly certain I have full control of the statement since I'm 
in SEE.


___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-03 Thread Jay A. Kreibich 
On Tue, Jul 03, 2012 at 04:26:42PM -0400, Richard Hipp scratched on the wall:

> Actually, you can bind on a DDL statement, but bindings are only valid for
> the lifetime of the statement itself, not for the whole lifetime of the
> object created by the CREATE statement.

  Is that a side-effect of the fact that CREATE statements are copied
  into the sqlite_master table as literals, and not re-written?  (Is
  that even true?)

   -j

-- 
Jay A. Kreibich < J A Y  @  K R E I B I.C H >

"Intelligence is like underwear: it is important that you have it,
 but showing it to the wrong people has the tendency to make them
 feel uncomfortable." -- Angela Johnson
___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-03 Thread Richard Hipp 
On Tue, Jul 3, 2012 at 4:18 PM, Yuriy Kaminskiy  wrote:

> Igor Tandetnik wrote:
> > On 7/3/2012 10:05 AM, Unsupported wrote:
> >>  // case 1: exception
> >>  //verify(sqlite3_prepare_v2(db, "create trigger updater
> >> update of result on plugins"
> >>  //  " begin"
> >>  //  " update mails set kav=case old.result when
> >> 'infected' then ? else 0 end where uid=old.uid;"
> >>  //  " end;"
> >>  //  , -1, , 0) == SQLITE_OK);
> >>  //verify(sqlite3_bind_int(stmt, ++idx, -1) == SQLITE_OK);
> >
> > You can't use parameters with DDL statements (all kinds of CREATE
> > SOMETHING, DROP SOMETHING et al). You can only parameterize DML
> > statements - SELECT, INSERT, UPDATE and DELETE.
>
> Hmm... I don't think this is incorrect per se, but where is this
> documented?
> I have not found any warning about this in prepare/bind/lang_expr/...
> documentation.
>

Igor is correct.

Actually, you can bind on a DDL statement, but bindings are only valid for
the lifetime of the statement itself, not for the whole lifetime of the
object created by the CREATE statement.  So doing such bindings are
pointless.  And you can't really bind to a DROP statement, because there
are no expressions in a DROP statement to bind to.  But these are all
details.  Igor's explanation is simple and to the point.

Yuriy is also correct that I need to update the documentation to make this
clear.  Or, maybe even throw an error at prepare-time or run-time if you
attempt to bind on a CREATE statement, so that people to don't normally
ready the documentation will also become aware of the limitation.


>
> ___
> sqlite-users mailing list
> sqlite-users@sqlite.org
> http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
>



-- 
D. Richard Hipp
d...@sqlite.org
___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-03 Thread Yuriy Kaminskiy 
Igor Tandetnik wrote:
> On 7/3/2012 10:05 AM, Unsupported wrote:
>>  // case 1: exception
>>  //verify(sqlite3_prepare_v2(db, "create trigger updater
>> update of result on plugins"
>>  //  " begin"
>>  //  " update mails set kav=case old.result when
>> 'infected' then ? else 0 end where uid=old.uid;"
>>  //  " end;"
>>  //  , -1, , 0) == SQLITE_OK);
>>  //verify(sqlite3_bind_int(stmt, ++idx, -1) == SQLITE_OK);
> 
> You can't use parameters with DDL statements (all kinds of CREATE
> SOMETHING, DROP SOMETHING et al). You can only parameterize DML
> statements - SELECT, INSERT, UPDATE and DELETE.

Hmm... I don't think this is incorrect per se, but where is this documented?
I have not found any warning about this in prepare/bind/lang_expr/... 
documentation.

___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Bug in sqlite3_step

2012-07-03 Thread Igor Tandetnik 

On 7/3/2012 10:05 AM, Unsupported wrote:

 // case 1: exception
 //verify(sqlite3_prepare_v2(db, "create trigger updater update of result on 
plugins"
 //  " begin"
 //  " update mails set kav=case old.result when 'infected' then ? else 
0 end where uid=old.uid;"
 //  " end;"
 //  , -1, , 0) == SQLITE_OK);
 //verify(sqlite3_bind_int(stmt, ++idx, -1) == SQLITE_OK);


You can't use parameters with DDL statements (all kinds of CREATE 
SOMETHING, DROP SOMETHING et al). You can only parameterize DML 
statements - SELECT, INSERT, UPDATE and DELETE.

--
Igor Tandetnik

___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

[sqlite] Bug in sqlite3_step

2012-07-03 Thread Unsupported 
Hello.

It looks like I found a bug in sqlite3_step. Here is the sample
code.

#include 
#include 
#include 
#include "sqlite3.h"

#ifdef _DEBUG
#define verify(f)  assert(f)
#else
#define verify(f)  ((void)(f))
#endif

char const * const fname = "bug.db3";

int cb(void*, int, char**, char**)
{
// If here, then got kav=-1 for uid=2
printf("ok");
return 0;
}

int main()
{
sqlite3* db = 0;
char* err;

DeleteFileA(fname);

verify(sqlite3_open(fname, ) == SQLITE_OK);

verify(sqlite3_exec(db, "drop table if exists mails;", 0, 0, ) == 
SQLITE_OK);
verify(sqlite3_exec(db, "drop table if exists plugins;", 0, 0, ) == 
SQLITE_OK);

verify(sqlite3_exec(db
, "create table if not exists mails ("
"uid integer not null primary key unique"
", kav integer not null default 0"
");"
, 0, 0, ) == SQLITE_OK);

verify(sqlite3_exec(db
, "create table plugins ("
"uid integer not null"
", plugin varchar(32) not null"
", result varchar(32) not null"
", primary key (uid, plugin));"
, 0, 0, ) == SQLITE_OK);

verify(sqlite3_exec(db, "insert into mails (uid) values (1);", 0, 0, 
) == SQLITE_OK);
verify(sqlite3_exec(db, "insert into mails (uid) values (2);", 0, 0, 
) == SQLITE_OK);
verify(sqlite3_exec(db, "insert into plugins (uid, plugin, result) 
values (1, 'kav', 'clean');", 0, 0, ) == SQLITE_OK);
verify(sqlite3_exec(db, "insert into plugins (uid, plugin, result) 
values (2, 'kav', 'infected');", 0, 0, ) == SQLITE_OK);

sqlite3_stmt* stmt = 0;

#if 0

// Work's correctly 
verify(sqlite3_prepare_v2(db, "create trigger updater update of result 
on plugins"
" begin"
" update mails set kav=case old.result when 'infected' then -1 
else 0 end where uid=old.uid;"
" end;"
, -1, , 0) == SQLITE_OK);

// In this case I get kav=-1 for uid=2 in mails table
#else

int idx = 0;

// case 1: exception
//verify(sqlite3_prepare_v2(db, "create trigger updater update of 
result on plugins"
//  " begin"
//  " update mails set kav=case old.result when 'infected' then ? 
else 0 end where uid=old.uid;"
//  " end;"
//  , -1, , 0) == SQLITE_OK);
//verify(sqlite3_bind_int(stmt, ++idx, -1) == SQLITE_OK);

// case 2: exception
//verify(sqlite3_prepare_v2(db, "create trigger updater update of 
result on plugins"
//  " begin"
//  " update mails set kav=case old.result when 'infected' then -1 
else ? end where uid=old.uid;"
//  " end;"
//  , -1, , 0) == SQLITE_OK);
//verify(sqlite3_bind_int(stmt, ++idx, 0) == SQLITE_OK);

// case 3: nothing happens
//verify(sqlite3_prepare_v2(db, "create trigger updater update of 
result on plugins"
//  " begin"
//  " update mails set kav=case old.result when ? then -1 else 0 
end where uid=old.uid;"
//  " end;"
//  , -1, , 0) == SQLITE_OK);
//verify(sqlite3_bind_text(stmt, ++idx, "infected", -1, SQLITE_STATIC) 
== SQLITE_OK);

// case 4: nothing happens
verify(sqlite3_prepare_v2(db, "create trigger updater update of result 
on plugins"
" begin"
" update mails set kav=case old.result when ? then ? else 0 end 
where uid=old.uid;"
" end;"
, -1, , 0) == SQLITE_OK);

verify(sqlite3_bind_text(stmt, ++idx, "infected", -1, SQLITE_STATIC) == 
SQLITE_OK);
verify(sqlite3_bind_int(stmt, ++idx, -1) == SQLITE_OK);

// In this case I get kav=0 for uid=2 in mails table

#endif

verify(sqlite3_step(stmt) == SQLITE_DONE);
verify(sqlite3_finalize(stmt) == SQLITE_OK);

verify(sqlite3_exec(db, "update plugins set result=result;", 0, 0, 
) == SQLITE_OK);

verify(sqlite3_exec(db, "select kav from mails where kav=-1;", cb, 0, 
) == SQLITE_OK);
verify(sqlite3_close(db) == SQLITE_OK);

return 0;
}

-- 
Best regards,
 Unsupported  mailto:unsuppor...@mail.ru

___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users