You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@apr.apache.org by bu...@apache.org on 2012/08/06 22:23:51 UTC
[Bug 53666] New: The Sybase/FreeTDS driver is broken -- misparses
the queries
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
Priority: P2
Bug ID: 53666
Assignee: bugs@apr.apache.org
Summary: The Sybase/FreeTDS driver is broken -- misparses the
queries
Severity: critical
Classification: Unclassified
OS: All
Reporter: mi+apache@aldan.algebra.com
Hardware: All
Status: NEW
Version: 1.4.1
Component: APR-util
Product: APR
While trying to configure some Sybase DB-backed redirections, I noticed, that
ALL of my queries were failing, even when the reported keys were present in the
queried table.
Digging deep into the dbd/dbd_freetds, I noticed, that the specified query
never makes it to the freetds-driver's code. For example, the driver's
dbd_freetds_prepare() invoked by apr_dbd_prepare() sees the line, that was
already "munched" by the caller.
In the below output from gdb, notice the difference between query in frame #1
and #2:
#1 0x00002aaab11a866d in dbd_freetds_prepare (pool=0x2aaab32c3178,
sql=0x2aaab32c1260,
query=0x2aaab32c32e0 "SELECT seo_url FROM CT_SEO WHERE vgn_url = ''",
label=0x2aaaaae082f0 "map_v2s", nargs=1, nvals=1, types=0x2aaab32c3320,
statement=0x7fffffffe0f0) at dbd/apr_dbd_freetds.c:493
stmt = 0x2aaab32c3328
#2 0x00002aaaab19a523 in apr_dbd_prepare (driver=0x2aaab13a97e0,
pool=0x2aaab32c3178, handle=0x2aaab32c1260,
query=0x2aaaaae08304 "SELECT seo_url FROM CT_SEO WHERE vgn_url = '%s'",
label=0x2aaaaae082f0 "map_v2s", statement=0x7fffffffe0f0)
at dbd/apr_dbd.c:476
qlen = 62
i = 1
nargs = 1
nvals = 1
p = 0x2aaab32c330d ""
pq = 0x2aaab32c32e0 "SELECT seo_url FROM CT_SEO WHERE vgn_url = ''"
q = 0x2aaaaae08333 ""
t = 0x2aaab32c3320
Perhaps, the driver should not be counting the args (nargs) on its own, relying
on the apr_dbd_prepare to provide the count?..
Whatever it is, the current latest RELEASE of apr-util's dbd is broken in this
regard :-(
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
--- Comment #10 from Mikhail T. <mi...@aldan.algebra.com> ---
Created attachment 29992
--> https://issues.apache.org/bugzilla/attachment.cgi?id=29992&action=edit
The same patch but against APR Util 1.5.1
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
--- Comment #4 from Mikhail T. <mi...@aldan.algebra.com> ---
Created attachment 29204
--> https://issues.apache.org/bugzilla/attachment.cgi?id=29204&action=edit
Fix/improve the FreeTDS-driver
The patch contains the following fix/improvements:
* Compiles against both FreeTDS and Sybase's own OpenClient headers
* Fixes access of uninitialized memory, when parsing the driver's parameters
(The above two resolve the Bug 53676)
* Removes remnants of the driver's own attempts to parse the statement relying
instead on the caller (apr_dbd_prepare()) to insert magical '\1' characters
into the placeholders for us
* Speeds-up creating actual statements from the pre-processed templates
* Fixes extraction cells to work with both Sybase and FreeTDS libraries (use
dbdatlen() instead of dbcollen())
* Records and makes available errors/messages
* Removes the attempts to "untaint" the arguments -- this should, probably, be
done by the callers and -- when the input data is untrusted -- mitigated by the
database-permissions
Potential problems:
* New code may be mixing tabs and spaces not in accordance with APU's current
coding style
* The new error-handling code may be too cavalier in allocating strings from
the pool(s). This should be infrequent, but, if the caller keeps submitting
erroneous statements using the same handle, the handle's pool may grow too
large -- help wanted.
* On a few (rare) occasions the new code will use fprintf(stderr) -- in the
belief, that outputting such messages somewhere is still better, than ignoring
them completely (and leaving the user wonder, why a login does not work, for
example)
* Parsing of the statement template relies on the unprintable character '\1' to
be inserted into the placeholders for us by the apr_dbd_prepare(). I would much
rather reimplement this using the proposal in Bug 53689 instead
Comfortable reassurance:
* With these changes the module (tested against both FreeTDS and Sybase) passes
most of the test/dbd.c (which needs to be modified as per Bug 53687) -- while
remaining clean under valgrind:
% valgrind --show-reachable=yes --leak-check=full --track-origins=yes test/dbd
freetds
server=v6_icstest_nj2x,host=s605202nj2sl267.uswhwk6.savvis.net,username=testV6icsuser,password=tV6icsuser,port=6200,charset=iso15,lang=us_english
==27683== Memcheck, a memory error detector
==27683== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==27683== Using Valgrind-3.5.0 and LibVEX; rerun with -h for copyright info
==27683== Command: test/dbd freetds
server=v6_icstest_nj2x,host=s605202nj2sl267,username=XXXX,password=XXX,port=6200,charset=iso15,lang=us_english
==27683==
Loaded freetds driver OK.
Name too long for LOGINREC field
v6_test_nj2x: Changed database context to 'v6content'.
Opened
freetds[server=v6_icstest_nj2x,host=s605202nj2sl267,username=XXXX,password=XXX,port=6200,charset=iso15,lang=us_english]
OK
======== create table ========
create table test successful
======== insert rows ========
insert rows test successful
======== invalid op ========
invalid op returned 1 (should be nonzero). Error msg follows
'v6_test_nj2x: apr_dbd_test1 not found. Specify owner.objectname or use sp_help
to check whether the object exists (sp_help may produce lots of output).
Line 1
208: General SQL Server error: Check messages from the SQL Server'
valid op returned 0 (should be zero; error shouldn't affect subsequent ops)
invalid op test successful
======== select random ========
get_row failed: NO_MORE_ROWS (DBBUFFER option must be on for dbgetrow() to
work)
Error in select random: rc=-1
======== select sequential ========
ROW 1: asdfgh bar 1
ROW 2: bar foo
ROW 3: foo
ROW 4: qwerty foo 0
ROW 5: wibble nothing 5
ROW 6: wibble other 5
select sequential test successful
======== transactions ========
Transaction 1
1 rows updated
Valid insert returned 0. Should be nonzero (fail) because transaction is bad
Transaction ended (should be rollback) - viewing table
A column of "failed" indicates transaction failed (no rollback)
ROW 1: asdfgh failed 1
ROW 2: bar failed
ROW 3: foo failed
ROW 4: qwerty failed 0
ROW 5: wibble failed 5
ROW 6: wibble failed 5
ROW 7: zzz aaa 3
Transaction 2
1 rows updated
Valid insert returned 0. Should be zero (OK)
Transaction ended (should be commit) - viewing table
ROW 1: aaa zzz 3
ROW 2: asdfgh success 1
ROW 3: bar success
ROW 4: foo success
ROW 5: qwerty success 0
ROW 6: wibble success 5
ROW 7: wibble success 5
ROW 8: zzz success 3
transactions test successful
======== prepared select ========
Selecting rows where col3 <= 3 and bar row where it's unset.
Should show four rows.
ROW 1: qwerty success 0
ROW 2: asdfgh success 1
ROW 3: bar success
ROW 4: zzz success 3
ROW 5: aaa zzz 3
prepared select test successful
======== prepared query ========
Showing table (should now contain row "prepared insert 2")
ROW 1: aaa zzz 3
ROW 2: asdfgh success 1
ROW 3: bar success
ROW 4: foo success
ROW 5: prepared insert 2
ROW 6: qwerty success 0
ROW 7: wibble success 5
ROW 8: wibble success 5
ROW 9: zzz success 3
prepared query test successful
======== drop table ========
drop table test successful
==27683==
==27683== HEAP SUMMARY:
==27683== in use at exit: 320 bytes in 2 blocks
==27683== total heap usage: 514 allocs, 512 frees, 345,428 bytes allocated
==27683==
==27683== 32 bytes in 1 blocks are still reachable in loss record 1 of 2
==27683== at 0x4A05430: calloc (vg_replace_malloc.c:418)
==27683== by 0x350180156A: _dlerror_run (in /lib64/libdl-2.5.so)
==27683== by 0x3501800F10: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.5.so)
==27683== by 0x4E65339: apr_dso_load (dso.c:139)
==27683== by 0x4C2C442: apu_dso_load (apu_dso.c:164)
==27683== by 0x4C1C97B: apr_dbd_get_driver (apr_dbd.c:195)
==27683== by 0x401E57: main (dbd.c:359)
==27683==
==27683== 288 bytes in 1 blocks are still reachable in loss record 2 of 2
==27683== at 0x4A0610C: malloc (vg_replace_malloc.c:195)
==27683== by 0x3500C10C4B: add_to_global (in /lib64/ld-2.5.so)
==27683== by 0x3500C110C8: dl_open_worker (in /lib64/ld-2.5.so)
==27683== by 0x3500C0D075: _dl_catch_error (in /lib64/ld-2.5.so)
==27683== by 0x3500C107EB: _dl_open (in /lib64/ld-2.5.so)
==27683== by 0x3501800F99: dlopen_doit (in /lib64/libdl-2.5.so)
==27683== by 0x3500C0D075: _dl_catch_error (in /lib64/ld-2.5.so)
==27683== by 0x350180150C: _dlerror_run (in /lib64/libdl-2.5.so)
==27683== by 0x3501800F10: dlopen@@GLIBC_2.2.5 (in /lib64/libdl-2.5.so)
==27683== by 0x4E65339: apr_dso_load (dso.c:139)
==27683== by 0x4C2C442: apu_dso_load (apu_dso.c:164)
==27683== by 0x4C1C97B: apr_dbd_get_driver (apr_dbd.c:195)
==27683==
==27683== LEAK SUMMARY:
==27683== definitely lost: 0 bytes in 0 blocks
==27683== indirectly lost: 0 bytes in 0 blocks
==27683== possibly lost: 0 bytes in 0 blocks
==27683== still reachable: 320 bytes in 2 blocks
==27683== suppressed: 0 bytes in 0 blocks
==27683==
==27683== For counts of detected and suppressed errors, rerun with: -v
==27683== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 35 from 7)
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
--- Comment #9 from Mikhail T. <mi...@aldan.algebra.com> ---
FYI: The patch is now part of the FreeBSD port devel/apr1
My own apr-dbd using application now works on FreeBSD in addition to Linux.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
--- Comment #1 from Mikhail T. <mi...@aldan.algebra.com> ---
FWIW, the same problem persists in APR/APU's current trunk...
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
--- Comment #3 from Mikhail T. <mi...@aldan.algebra.com> ---
No, that is not sufficient. Even running the
dbd/test freetds ....
crashes on "prepared select":
======== prepared select ========
Program received signal SIGSEGV, Segmentation fault.
0x0000003501079b80 in strlen () from /lib64/libc.so.6
(gdb) where
#0 0x0000003501079b80 in strlen () from /lib64/libc.so.6
#1 0x00002aaaab143a89 in dbd_statement (pool=0x608128, stmt=0x608780, nargs=1,
args=0x6087f0) at dbd/apr_dbd_freetds.c:220
#2 0x00002aaaab143c2f in dbd_freetds_pselect (pool=0x608128, sql=0x608200,
results=0x7fffffffe500, statement=0x608780, seek=0, values=0x6087f0) at
dbd/apr_dbd_freetds.c:248
#3 0x00002aaaab143d68 in dbd_freetds_pvselect (pool=0x608128, sql=0x608200,
results=0x7fffffffe500, statement=0x608780, seek=0, args=0x7fffffffe3d0) at
dbd/apr_dbd_freetds.c:270
#4 0x00002aaaaaac27c8 in apr_dbd_pvselect (driver=0x2aaaab345760,
pool=0x608128, handle=0x608200, res=0x7fffffffe500, statement=0x608780,
random=0) at dbd/apr_dbd.c:519
#5 0x0000000000401a97 in test_pselect (pool=0x608128, handle=0x608200,
driver=0x2aaaab345760) at dbd.c:287
#6 0x000000000040232e in main (argc=3, argv=0x7fffffffe688) at dbd.c:395
(gdb) up
#1 0x00002aaaab143a89 in dbd_statement (pool=0x608128, stmt=0x608780, nargs=1,
args=0x6087f0) at dbd/apr_dbd_freetds.c:220
220 len = strlen(stmt->fmt) +1;
(gdb) p stmt->fmt
$1 = 0x73000000006087a0 <Address 0x73000000006087a0 out of bounds>
The fmt is corrupted by earlier attempts at regexp-matching and string-moving
(actual culprit is the memmove call in recurse_args()) -- neither are necessary
at all...
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
Mikhail T. <mi...@aldan.algebra.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #29204|0 |1
is obsolete| |
--- Comment #6 from Mikhail T. <mi...@aldan.algebra.com> ---
Created attachment 29807
--> https://issues.apache.org/bugzilla/attachment.cgi?id=29807&action=edit
Fix/improve the FreeTDS-driver
Additional improvements/fixes for items discovered during ongoing use of the
driver.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
Mikhail T. <mi...@aldan.algebra.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |PatchAvailable
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
--- Comment #2 from Bojan Smojver <bo...@rexursive.com> ---
Possible fix (completely untested):
Index: apr_dbd_freetds.c
===================================================================
--- apr_dbd_freetds.c (revision 1370619)
+++ apr_dbd_freetds.c (working copy)
@@ -794,7 +794,7 @@
dbd_freetds_get_name,
dbd_freetds_transaction_mode_get,
dbd_freetds_transaction_mode_set,
- "",
+ "%%s",
dbd_freetds_pvbquery,
dbd_freetds_pvbselect,
dbd_freetds_pbquery,
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
--- Comment #8 from Mikhail T. <mi...@aldan.algebra.com> ---
(In reply to comment #7)
> I can believe that it's broken, especially in the parts marked as
> unimplemented.
It is broken beyond belief. The global/driver API has changed since this driver
was written and it was never brought up to date. The changes made to it allowed
it to compile, but not work.
Try it, if you dare :-)
> But why does your patch remove all the untainting code?
No other driver is doing it, is one reason. It is also expensive (applying a
regexp for each query) -- and not necessary, see below. But most importantly,
because it may reject valid (and useful code): depending on application --
apr_dbd stuff can be used for purposes other than a read-only lookup from
inside httpd.
> Can you explain, for example, how a user of mod_authn_dbd executes the
> standard user lookup query without opening the server to all kinds of
> SQL injection attack?
By setting up -- and using -- a special database account whose
access-permissions only allow it to SELECT from certain tables or, better yet,
to only EXEC certain stored procedures. This is the only method guaranteed to
work anyway...
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
--- Comment #5 from Mikhail T. <mi...@aldan.algebra.com> ---
*** Bug 53676 has been marked as a duplicate of this bug. ***
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
--- Comment #7 from Nick Kew <ni...@webthing.com> ---
I can believe that it's broken, especially in the parts marked as
unimplemented.
But why does your patch remove all the untainting code? Can you explain, for
example, how a user of mod_authn_dbd executes the standard user lookup query
without opening the server to all kinds of SQL injection attack?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org
[Bug 53666] The Sybase/FreeTDS driver is broken -- misparses the
queries
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53666
Mikhail T. <mi...@aldan.algebra.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Depends on| |53687
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org