Mailing-List: contact empire-db-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: empire-db-dev@incubator.apache.org
Received-SPF: pass (athena.apache.org: local policy)
Content-class: urn:content-classes:message
Subject: re: Type checks in DBCommand in order to avoid SQL Injection
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Date: Thu, 28 Jan 2010 21:40:19 +0100
Message-ID: <B297D4737108A0459B66C3A85BB7F9011479B6@kn.esteam.de>
In-Reply-To: <a747219a1001261622q6cfcc60dy60bad3ab84753fa4@mail.gmail.com>
Thread-Topic: re: Type checks in DBCommand in order to avoid SQL Injection
Thread-Index: Acqe5tWhuy3a6WYdRhKPRC4ub2IP4gBcUduA
References: <a747219a1001180731q4d793c6bt67ee77d1bd70395@mail.gmail.com>
 <a747219a1001191515y5e376cbclc96e981ee3ec35d8@mail.gmail.com>
 <a747219a1001200420x48b8c134n8eacb236e266008d@mail.gmail.com>
 <B297D4737108A0459B66C3A85BB7F901147961@kn.esteam.de>
 <a747219a1001240709k37ca944ta5c6f21f5feee5ec@mail.gmail.com>
 <B297D4737108A0459B66C3A85BB7F901147962@kn.esteam.de>
 <a747219a1001241256p2de652fqb565429b2e8753ee@mail.gmail.com>
 <B297D4737108A0459B66C3A85BB7F901147963@kn.esteam.de>
 <a747219a1001241402o3082cfd7la07620fbccbdd432@mail.gmail.com>
 <B297D4737108A0459B66C3A85BB7F901147991@kn.esteam.de>
 <a747219a1001261622q6cfcc60dy60bad3ab84753fa4@mail.gmail.com>
From: =?iso-8859-1?Q?Rainer_D=F6bele?= <doebele@esteam.de>
To: <empire-db-dev@incubator.apache.org>

Hi McKinley,

to be honest: I don't think what you want makes sense and it's not what =
we're trying to achieve.
Empire-db should be lightweight plus easy and straightforward to use.
Adding various options adds complexity and is not intuitive.
And please understand that we cannot simply add features if someone has =
one very specific problem.

The root of your problem is that you using a parameter of a web-request =
directly as the constraint of a query.
This is not a good idea.
Anyone can forge the request and even if you have a check for the proper =
data type, this person could access any record even if he had no =
credentials for that record. The check for validity can only be =
performed by the application on a higher level at which you would in =
this case need to convert the string to an integer (or something else) =
anyway. At least you should.

So let's try keeping Empire-db slim and simple.
Otherwise we end up as a hibernate clone :-)

Regards
Rainer

BTW: You should really have a look at our Web-examples. For forms they =
use DBRecord objects which all kinds of validation checks (not just the =
data type). But I must admit apart from that there is not enough =
security built in either. Security is a issue of its own.

McKinley wrote:
> Re: Type checks in DBCommand in order to avoid SQL Injection
>=20
> 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 =3D '0;
> update this set that=3D\'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.
>=20
> 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.
>=20
> 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.
>=20
> Thanks,
>=20
> McKinley
>=20
>=20
> On Tue, Jan 26, 2010 at 11:37 PM, Rainer D=F6bele <doebele@esteam.de>
> 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