Re: Bug in sql escaping in apr_dbd_sqlite2 and apr_dbd_sqlite3

[Ronen Mizrahi <ro...@tversity.com>]

Any comments to this?

I apologize for the extra * charcaters in the code snippets from my 
first email, let me try again. The current code in apr_dbd_sqlite2.c and 
in apr_dbd_sqlite3.c for escaping strings is:

static const char *dbd_sqlite3_escape(apr_pool_t *pool, const char *arg,
                                      apr_dbd_t *sql)
{
    char *ret = sqlite3_mprintf(arg);
    apr_pool_cleanup_register(pool, ret, (void *) sqlite3_free,
                              apr_pool_cleanup_null);
    return ret;
}

The first line of the function need to be changed to:
    char *ret = sqlite3_mprintf("%q", arg);

Otherwise the use of '%' charcaters in arg will have unwanted side effects.

Ronen Mizrahi wrote:

> *The following code (used both in apr_dbd_sqlite2.c and in 
> apr_dbd_sqlite3.c) in order to escaqpe SQL strings is incorrect.
> When the % charcater appears in the arg it is misniterpreted of-course 
> and can have far reaching side effects.
> The proper solution is listed below as well.
>
> INCORRECT:
> static* *const* *char* **dbd_sqlite3_escape*(apr_pool_t *pool, *const* 
> *char* *arg,
>                                      apr_dbd_t *sql)
> {
>    *char* *ret = sqlite3_mprintf(arg);
>    apr_pool_cleanup_register(pool, ret, (*void* *) sqlite3_free,
>                              apr_pool_cleanup_null);
>    *return* ret;
> }
>
>
> *CORRECT:
> static* *const* *char* **dbd_sqlite3_escape*(apr_pool_t *pool, *const* 
> *char* *arg,
>                                      apr_dbd_t *sql)
> {
>    *char* *ret = sqlite3_mprintf("%q", arg);
>    apr_pool_cleanup_register(pool, ret, (*void* *) sqlite3_free,
>                              apr_pool_cleanup_null);
>    *return* ret;
> }
>
>

Re: Bug in sql escaping in apr_dbd_sqlite2 and apr_dbd_sqlite3

[Garrett Rooney <ro...@electricjellyfish.net>]

On 1/12/06, Ronen Mizrahi <ro...@tversity.com> wrote:
> Sending a patch might take some time since I am currently using an older
> version of APR. I did verify however that the issue still exists in the
> SVN head.

Fix (plus a simple test) committed in r372724.

Thanks,

-garrett

Re: Bug in sql escaping in apr_dbd_sqlite2 and apr_dbd_sqlite3

[Ronen Mizrahi <ro...@tversity.com>]

Sending a patch might take some time since I am currently using an older 
version of APR. I did verify however that the issue still exists in the 
SVN head.

Garrett Rooney wrote:

>On 1/12/06, Ronen Mizrahi <ro...@tversity.com> wrote:
>  
>
>>Any comments to this?
>>    
>>
>
>This is on my todo list, I just haven't gotten around to it.  One
>thing that would be helpful would be if you'd send an actual patch
>that fixes the problem, instead of just a description of what should
>be changed.  Bonus points for a test case that shows the issue ;-)
>
>-garrett
>
>  
>

Re: Bug in sql escaping in apr_dbd_sqlite2 and apr_dbd_sqlite3

[Garrett Rooney <ro...@electricjellyfish.net>]

On 1/12/06, Ronen Mizrahi <ro...@tversity.com> wrote:
> Any comments to this?

This is on my todo list, I just haven't gotten around to it.  One
thing that would be helpful would be if you'd send an actual patch
that fixes the problem, instead of just a description of what should
be changed.  Bonus points for a test case that shows the issue ;-)

-garrett