Would the check go into that the addSQL() -> getObjectValue(expr.getDataType()... chain? In the case of my example, I have passed in a string so rather than give too much deference to the column data type, why not say "rare is the DBMS that will cast this correctly, but the developer seems to want [where intCol1 = '0; update this set that=\'bla\';']. Of course this requires adding a bunch of type checks on that Object value that was passed to determine that it is a string and should be quoted. I don't think I want any parseInt or regexp for ';' and '--'. Just quote wrapping when the value is character based.
If you do add checks how about changing addSql() to addSql(boolean safe)? addSql() would do any checks, but if you have white-listed all your inputs you can use addSql(false) to get extra speed. I guess another option is that checks could be put into the DBCompareColExpr generators like is() and isBetween() so that they take specific types instead of Object, but that might be too much boilerplate code. Thanks, McKinley On Tue, Jan 26, 2010 at 11:37 PM, Rainer Döbele <[email protected]> wrote: > Hi McKinley > > Just to let you know: > In the Where clause of the DBCommand object we're not doing any type checks > at all. > Hence your SQL-Injection example below also works for other numeric columns. > > The question is whether we should at this point or not. > We're doing type-checks when working with DBRecord objects. > They internally then use a DBCommand as well. > Adding a check in DBCommand would mean a double check. > > I think the DBCommand is the lowest level. > All checks should be performed at a higher level in order to avoid Overhead. > > Regards > Rainer
