Bug 8232 - smbtad SQL statements need to be escaped.
Summary: smbtad SQL statements need to be escaped.
Status: RESOLVED FIXED
Alias: None
Product: smbta
Classification: Unclassified
Component: smbtad (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal
Target Milestone: ---
Assignee: Holger Hetterich
QA Contact:
URL:
Keywords:
Depends on: 8205
Blocks: 8204
  Show dependency treegraph
 
Reported: 2011-06-14 10:33 UTC by Robert Piasek (dead mail address)
Modified: 2011-09-06 15:55 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Piasek (dead mail address) 2011-06-14 10:33:50 UTC
Statements with single quites ' in file/directory names cause problems.

They need to be escaped.

STATEMENT:  INSERT INTO open (username, usersid, share, domain, timestamp,filename, mode, result) VALUES ('USER1','S-1-5-21-1566210211-763273816-3362882449-1192','Photos','AD','2011-05-09 18:12:07.756','Shortcut to retail on 'NAS1 (nas1)' (Q).lnk',496,38);STATEMENT:  INSERT INTO open (username, usersid, share, domain, timestamp,filename, mode, result) VALUES ('USER2','S-1-5-21-1566210211-763273816-3362882449-1270','Tech','AD','2011-06-12 09:54:14.463','Shoot'em',496,33);

http://imageshack.us/f/269/115pp.png/
http://imageshack.us/f/217/116dkb.png/

Suggestion:

libdbi 0.8.3 (the "Steeltown" release)

Bugfixes
...

Improvements

- three functions were added: dbi_conn_escape_string(),
  dbi_conn_escape_string_copy(), and dbi_conn_escape_binary_copy()
  perform the same escaping of special characters as the corresponding
  *_quote_* functions do, but they do not surround the resulting
  string with quotes. This may at times be more convenient if the
  escaped strings are not directly inserted into a SQL query. 


dbi_conn_escape_string sounds just about right. That would also bump libdbi requirements to 0.8.3
Comment 1 Holger Hetterich 2011-06-15 21:07:38 UTC
Yes, these functions will do the job. 

todo

- add a test for version 0.8.3 of libdbi to the cmakefiles
- first fix/implement bug 8205, looking at smbtad database handling functions, it'll be much easier when 8205 is implemented.
Comment 2 Robert Piasek (dead mail address) 2011-06-16 09:28:38 UTC
Actually I'm already on libdbi 0.8.3 - been since the beginning and it's working just fine.
Comment 3 Holger Hetterich 2011-06-19 18:56:22 UTC
Robert, yes at the suse camp we are also at 0.8.3, but a test in cmakefiles is required to make sure we use this version whoever it compiles.
Comment 4 Holger Hetterich 2011-06-28 06:32:13 UTC
Once we have the new database setup working, we'll tackle this.
Comment 5 Holger Hetterich 2011-07-01 14:07:17 UTC
[devel 4623e08] Support quoting for the target database when a cache entry is interpreted for the database.


please test
Comment 6 Robert Piasek (dead mail address) 2011-07-01 15:11:08 UTC
4623e0885eaf0d4fc170534e9050321e2907b0e8 seems to fix escaping bug.

Based on very brief tests, there is no performance hit of any kid. More detailed tests will follow. Memory allocation remain very very low.

All strings with ' characters are successfully logged to the database.
Comment 7 Robert Piasek (dead mail address) 2011-07-01 16:28:14 UTC
...
Running Test 81722th time...
Running Test 81723th time...
Running Test 81724th time...
Running Test 81725th time...
....

all worker perfectly fine. Run over 120000 tests and all fine. Right now I'm working on generating some stats to see how much performance impact is has.

Rob
Comment 8 Holger Hetterich 2011-07-01 17:16:52 UTC
I somehow need to add a test for libdbi 0.8.3 but the library itself doesn't include version information like pkgconfig... Any clue? If not, I am writing a cmake test that will test for dbi_conn_quote* with a small program.
Comment 9 Robert Piasek (dead mail address) 2011-08-06 21:49:57 UTC
I'm not sure if we really have to worry about libdbi version. 0.8.3 was released on February 6, 2008 and I don't think any distro will ship anything older. Considering how new samba this will require, I wouldn't really worry about it at all.

Also all dependencies can be enforced by package manager.
Comment 10 Holger Hetterich 2011-09-06 15:55:54 UTC
Yes, I think so too. So this bug can be closed.