Re: empty values in INSERT INTO statement
- From: "Mark J. McGinty" <mmcginty@xxxxxxxxxxxxxxx>
- Date: Mon, 25 Apr 2005 03:27:30 -0700
Bob, I'm not trying to be overly negative, I know you post to these news
groups regularly and make a substantial contribution... But I felt I needed
to add something, and hope you won't take it badly.
"Bob Barrows [MVP]" <reb01501@xxxxxxxxxxxxxxx> wrote in message
news:Oyn%23Dx$RFHA.1348@xxxxxxxxxxxxxxxxxxxxxxx
[snip]
> 'Regardless of which option you are using start by validating inputs
>
> for each key in Request.Form
> s=lcase(request.form(key))
> 'The following should be down with regexp. This is a demo
> if instr(s,"select")>0 OR instr(s,"insert")>0 OR instr(s,"update")>0 _
> OR instr(s,"delete")>0 THEN
> 'punish the hacker
[snip]
If you're using parameters what's the point? Text inside a parameter will
not be executed as SQL, ever, plus those words could occur in valid input,
e.g., [Product Name] = 'Select Beef'. What's more, those InStr calls are
case-sensitive without some more [optional] arguments, while SQL is not, so
any variation of case would thwart this check.
What's even more, even without parameterizing those values, a SQL injection
would depend on at least some punctuation, such as a semi-colon or
double-dash (neither of which may work in Jet, don't know, don't care,
that's beside the point.) A SQL injection has to terminate the fragment of
the intended statement to the left of the injection, and preserve syntax in
spite of the fragment to the right of the injection (usually via the comment
character.) The whole statement, with injection has to be syntactically
valid, or the whole statement fails.
I'm not saying that input validation when using parameterized SQL is
unnecessary, but such needs are app-specific. Searching parameterized
values for possible SQL injection is pointless.
I'm also not inferring that this fragment was presented as a complete
validation solution, however, as examples of concept implementations go,
this one lacks critical functionality, and doesn't do much to illustrate the
underlying concept -- at minimum it should perform a case-insensitive test,
e,g.,
vbTextCompare = 1
[...]
if (InStr(1, s, "delete", vbTextCompare) > 0) Or ...
And ideally it should test for a more positive indication of attempted SQL
injection, because this level of test could easily return a false positive
for input that in actuality is valid as well as harmless.
Bottom line, any time you're thinking "punish the hacker," you're treading
on thin ice, and you'd best make damn sure there really is a hacker to
punish! And just as important [if not more so], if you have something
special planned for hackers, best make DAMN sure ALL of your security ducks
are in a row, because if you are unlucky enough to draw their attention,
you'll find out the hard way that they are relentless, and capable of
dishing out punishments of their own. It can be excruciating.
-Mark
.
- Follow-Ups:
- Re: empty values in INSERT INTO statement
- From: Bob Barrows [MVP]
- Re: empty values in INSERT INTO statement
- References:
- empty values in INSERT INTO statement
- From: TB
- Re: empty values in INSERT INTO statement
- From: Bob Barrows [MVP]
- Re: empty values in INSERT INTO statement
- From: TB
- Re: empty values in INSERT INTO statement
- From: Bob Barrows [MVP]
- empty values in INSERT INTO statement
- Prev by Date: Re: Upsizing issue
- Next by Date: Re: empty values in INSERT INTO statement
- Previous by thread: Re: empty values in INSERT INTO statement
- Next by thread: Re: empty values in INSERT INTO statement
- Index(es):
Relevant Pages
|
