Everything

php

Home » Project » Newsgroups » php.internals » PHP 4.3.3RC3 Released »

PHP 4.3.3RC3 Released

php.internals

Ilia A.

23 years ago

PHP 4.3.3RC3 was just released, hopefully this will be the final release
candidate prior to the final release. Please test this release as much
possible so that any new/critical bugs maybe uncovered and resolved before
the final. If you do find such bugs (hopefully you won't) be sure to label
them as RC3 bugs.

I would like to ask that all developers refrain from making commits to the 4_3
tree until 4.3.3 final is released, unless a patch addresses a critical
issue. Critical issues are defined as the following:
1) Security Fixes
2) Fixes for bugs introduced in 4.3.3X releases
3) Fixes for bugs that break backwards compatibility with older versions.

Ilia

moshe doron

23 years ago

"Ilia Alshanetsky" <ilia@prohost.org> wrote in message
news:<200308071447.03285.ilia@prohost.org>...

> I would like to ask that all developers refrain from making commits to

> the 4_3 tree until 4.3.3 final is released, unless a patch addresses a

> critical issue. Critical issues are defined as the following:

> 1) Security Fixes

What about hacking somehow the sqlite library to disallow chained queries
(or at least do it optionally)?

This behavior is *huge* security hole, allow to the cracker drop ur database
using simple select where query.

moshe

Derick Rethans

23 years ago

On Thu, 14 Aug 2003, moshe doron wrote:

> What about hacking somehow the sqlite library to disallow chained queries
> (or at least do it optionally)?
>
> This behavior is *huge* security hole, allow to the cracker drop ur database
> using simple select where query.

How is this a security hole?

regards,
Derick

--
"Interpreting what the GPL actually means is a job best left to those
that read the future by examining animal entrails."
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
International PHP Magazine                          http://php-mag.net/
-------------------------------------------------------------------------

moshe doron

23 years ago

"Derick Rethans" <derick@php.net> wrote in message
news:Pine.LNX.4.53.0308140932320.12247@jdi.jdimedia.nl...

> On Thu, 14 Aug 2003, moshe doron wrote:
>
> > What about hacking somehow the sqlite library to disallow chained

queries

> > (or at least do it optionally)?
> >
> > This behavior is *huge* security hole, allow to the cracker drop ur

database

> > using simple select where query.
>
> How is this a security hole?

http://www.phpbuilder.com/mail/php-developer-list/2003022/0062.php

moshe

(Marcus Börger)

23 years ago

Hello moshe,

Thursday, August 14, 2003, 10:41:16 AM, you wrote:

md> "Derick Rethans" <derick@php.net> wrote in message
md> news:Pine.LNX.4.53.0308140932320.12247@jdi.jdimedia.nl...

>> On Thu, 14 Aug 2003, moshe doron wrote:
>>
>> > What about hacking somehow the sqlite library to disallow chained

md> queries

>> > (or at least do it optionally)?
>> >
>> > This behavior is *huge* security hole, allow to the cracker drop ur

md> database

>> > using simple select where query.
>>
>> How is this a security hole?

md> http://www.phpbuilder.com/mail/php-developer-list/2003022/0062.php

Bullshit.

If the cracker can change one of your sql statements he already has access to
your machine. In that case he wouldn't bother changing your sql statements.

--
Best regards,
Marcus                            mailto:helly@php.net

moshe doron

23 years ago

"Marcus BöRger" <marcus.boerger@t-online.de> wrote in message

> md> http://www.phpbuilder.com/mail/php-developer-list/2003022/0062.php
>
> Bullshit.
>
> If the cracker can change one of your sql statements he already has access

to

> your machine. In that case he wouldn't bother changing your sql

statements.

>

that's the point. if the cracker can change only the end of the query, it's
not so usefull for him (he can maximum get other id) but if he can chain
totally new query, he may or may no bother changing your sql statements....

moshe.

Derick Rethans

23 years ago

On Thu, 14 Aug 2003, moshe doron wrote:

>
> "Marcus BöRger" <marcus.boerger@t-online.de> wrote in message
> > md> http://www.phpbuilder.com/mail/php-developer-list/2003022/0062.php
> >
> > Bullshit.
> >
> > If the cracker can change one of your sql statements he already has access to
> > your machine. In that case he wouldn't bother changing your sql statements.
> >
>
> that's the point. if the cracker can change only the end of the query, it's
> not so usefull for him (he can maximum get other id) but if he can chain
> totally new query, he may or may no bother changing your sql statements....

But it's a *user* problem (the developer), not a PHP problem. PHP should
not break nice functionality in an extension (such as chaining queries)
because of people too lazy to verify user input.

Derick

--
"Interpreting what the GPL actually means is a job best left to those
that read the future by examining animal entrails."
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
International PHP Magazine                          http://php-mag.net/
-------------------------------------------------------------------------

Wez Furlong

23 years ago

+1.

This is no bug in PHP, it is not a security flaw in PHP; it is a problem
in your code.  PHP shouldn't police your lax security because you can't
be bothered with it.

Can we drop this thread now? :-)

--Wez.

On Thu, 14 Aug 2003, Derick Rethans wrote:

Steven Brown

23 years ago

> -----Original Message-----
> From: Derick Rethans [mailto:derick@php.net]
> Sent: Thursday, August 14, 2003 12:33 AM
> To: moshe doron
> Cc: internals@lists.php.net
> Subject: Re: [PHP-DEV] Re: PHP 4.3.3RC3 Released
>
>
> On Thu, 14 Aug 2003, moshe doron wrote:
>
> > What about hacking somehow the sqlite library to disallow chained
> > queries (or at least do it optionally)?
> >
> > This behavior is *huge* security hole, allow to the cracker drop ur
> > database using simple select where query.
>
> How is this a security hole?

E.g.,

$id = "0; drop bar";
lamesql_query("select foo from bar where id = $id");

moshe doron

23 years ago

"Steven Brown" <swbrown@ucsd.edu> wrote in message
news:000001c36237$f92a5410$1b0fa518@swbrown...

>
>
> > -----Original Message-----
> > From: Derick Rethans [mailto:derick@php.net]
> > Sent: Thursday, August 14, 2003 12:33 AM
> > To: moshe doron
> > Cc: internals@lists.php.net
> > Subject: Re: [PHP-DEV] Re: PHP 4.3.3RC3 Released
> >
> >
> > On Thu, 14 Aug 2003, moshe doron wrote:
> >
> > > What about hacking somehow the sqlite library to disallow chained
> > > queries (or at least do it optionally)?
> > >
> > > This behavior is *huge* security hole, allow to the cracker drop ur
> > > database using simple select where query.
> >
> > How is this a security hole?
>
> E.g.,
>
> $id = "0; drop bar";
> lamesql_query("select foo from bar where id = $id");
>

that's the mysql way....
with sybase style (used by sqlite), the is no need to separator, makes the
detection even harded.
(i used using pear sql_parser before executing the query but u can also turn
on sybase magic mode (worst choice ! it's buggy) and *always* encapsulate
the vars cames from the user with " ' ")

moshe.

Derick Rethans

23 years ago

On Thu, 14 Aug 2003, moshe doron wrote:

> > E.g.,
> >
> > $id = "0; drop bar";
> > lamesql_query("select foo from bar where id = $id");
> >
> that's the mysql way....
> with sybase style (used by sqlite), the is no need to separator, makes the
> detection even harded.

uhm? SQLite definitely needs a seperator there.

Derick

--
"Interpreting what the GPL actually means is a job best left to those
that read the future by examining animal entrails."
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
International PHP Magazine                          http://php-mag.net/
-------------------------------------------------------------------------

moshe doron

23 years ago

"Derick Rethans" <derick@php.net> wrote in message
news:Pine.LNX.4.53.0308141029380.18970@jdi.jdimedia.nl...

>
> uhm? SQLite definitely needs a seperator there.

yep, i got confused by the ' style (sybase one).

moshe.

Hartmut Holzgraefe

23 years ago

Steven Brown wrote:

>>How is this a security hole?
>
>
> E.g.,
>
> $id = "0; drop bar";
> lamesql_query("select foo from bar where id = $id");

this is also possible with oracle, oci8 and other database extensions,
mysql ist the only one where i'm sure about that chaining is *not*
supported

so the actual security hole is user code that puts *unverified* input
into SQL queries, so opening the door for SQL injection

whether it makes sense to disable command chaining or at least make
it configurable with default 'off' in PHP database extensions is a
topic that may need further discussion (i know that Georg has similar
plans for mysqli in PHP 5),
but claiming that command chaining is a '*huge*' security hole per se
is not justified IMHO, this is more about how much protection against
"shoot yourself in the foot" incidents PHP should offer ...

--
Hartmut Holzgraefe  <hartmut@php.net>

moshe doron

23 years ago

"Hartmut Holzgraefe" <hartmut@php.net> wrote in message
news:3F3B4142.2060007@php.net...

> Steven Brown wrote:
> >>How is this a security hole?
> >
> >
> > E.g.,
> >
> > $id = "0; drop bar";
> > lamesql_query("select foo from bar where id = $id");
>
> this is also possible with oracle, oci8 and other database extensions,
> mysql ist the only one where i'm sure about that chaining is *not*
> supported
>
> so the actual security hole is user code that puts *unverified* input
> into SQL queries, so opening the door for SQL injection
>
> whether it makes sense to disable command chaining or at least make
> it configurable with default 'off' in PHP database extensions is a
> topic that may need further discussion (i know that Georg has similar
> plans for mysqli in PHP 5),
> but claiming that command chaining is a '*huge*' security hole per se
> is not justified IMHO, this is more about how much protection against
> "shoot yourself in the foot" incidents PHP should offer ...
>

the question is what is the common php programing habit.
sinse there is no public recommendation on the manual (fix me here) or
someware else, i  assume thats the hebit is not always do verify the data
and u can't blame the php users.

btw, i doubt if u want to publish here the db sechema and url to system
running oracle in ur ownership ...


moshe.

Derick Rethans

23 years ago

On Thu, 14 Aug 2003, moshe doron wrote:

> the question is what is the common php programing habit.
> sinse there is no public recommendation on the manual (fix me here) or
> someware else, i  assume thats the hebit is not always do verify the data
> and u can't blame the php users.

Well, that's not our problem if people don't verify user input. Every
article dealing with the simplest security problem mentions this. I
don't think it's a good idea to break this functionality. (yes, even
MySQL has this in version 4.1+)

> btw, i doubt if u want to publish here the db sechema and url to system
> running oracle in ur ownership ...

"u" and "ur" are not in my dictionary, perhaps you mean "you" and "your"
?

Derick

--
"Interpreting what the GPL actually means is a job best left to those
that read the future by examining animal entrails."
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
International PHP Magazine                          http://php-mag.net/
-------------------------------------------------------------------------

Steven Brown

23 years ago

> -----Original Message-----
> From: Derick Rethans [mailto:derick@php.net]
> Sent: Thursday, August 14, 2003 1:29 AM
> To: moshe doron
> Cc: internals@lists.php.net
> Subject: Re: [PHP-DEV] Re: PHP 4.3.3RC3 Released
>
>
> On Thu, 14 Aug 2003, moshe doron wrote:
>
> > the question is what is the common php programing habit.
> sinse there
> > is no public recommendation on the manual (fix me here) or someware
> > else, i  assume thats the hebit is not always do verify the
> data and u
> > can't blame the php users.
>
> Well, that's not our problem if people don't verify user input. Every
> article dealing with the simplest security problem mentions this.

You mean verifying user input like this?  (My Outlook will wrap this URL
btw, sorry):

http://bugs.php.net/search.php?search_for=&boolean=0&limit=10&order_by=&
reorder_by=XXX&direction=ASC&cmd=display&status=Open&php_os=&phpver=&ass
ign=&author_email=&bug_age=0

"SELECT SQL_CALC_FOUND_ROWS *, TO_DAYS(NOW())-TO_DAYS(ts2) AS unchanged
FROM bugdb WHERE bug_type != 'Feature/Change Request' AND (status='Open'
OR status='Assigned' OR status='Analyzed' OR status='Critical' OR
status='Verified') AND (SUBSTRING(php_version,1,1) = '4' OR
SUBSTRING(php_version,1,1) = '5' OR php_version = 'Irrelevant') ORDER BY
XXX ASC LIMIT 0,10
Unknown column 'XXX' in 'order clause'"

php.net has some sort of failsafe it seems if you try and add another
semicolon, "BAD HACKER!! No database cracking for you today!", hee, but
you have to concede how easy it is to not properly check user input that
makes it into a query.  That took me all of 15 minutes to find.  It's
hard to find a large PHP-based site that doesn't have at least one such
bug, php.net included.

It's an obvious argument for not adding unsafe functionality when you
don't have to, isn't it? :)

Wez Furlong

23 years ago

No :)

If you are really worried about these kind of attacks, I suggest you
read one of the many articles that explains how to write a secure
PHP application.

You can also take it further and write a "user input" filter using the
framework that Rasmus added for this purpose (mostly from a XSS point of
view, but you could apply it to SQL injection also).

Lets kill this thread, as it is not an internals@ matter.

--Wez.

On Thu, 14 Aug 2003, Steven Brown wrote:

moshe doron

23 years ago

"Derick Rethans" <derick@php.net> wrote in message
news:Pine.LNX.4.53.0308141027490.18970@jdi.jdimedia.nl...

> On Thu, 14 Aug 2003, moshe doron wrote:
>
> > the question is what is the common php programing habit.
> > sinse there is no public recommendation on the manual (fix me here) or
> > someware else, i  assume thats the hebit is not always do verify the

data

> > and u can't blame the php users.
>
> Well, that's not our problem if people don't verify user input. Every
> article dealing with the simplest security problem mentions this. I
> don't think it's a good idea to break this functionality. (yes, even
> MySQL has this in version 4.1+)

Every application manager for application that grow beyond the traditional
simplest sample, know that the programers makes
this kind of errors, and lot of them, specially in this part of the software
where all run as usual even with the bugs.
the price for every simple and common error is one that php can't shake free
of responsibility and blame the users.

till today, the most common use with php was mysql, that have no this kind
of problem, and exotic and expensive one like oracle, mssql (i'm owner of
production application in both) doen't got much focus.
today with sqlite, the status is going to change and w'll see more and more
"unverified" but open to public sites with this kind of error. and yes,
1) help users not shouting themself.
2) give the users the choise to keep things simple (not verify at all,
specially in the context of the typeless sqlite...)
is are good things of php, and there is no reason not continuing.

> > btw, i doubt if u want to publish here the db sechema and url to system
> > running oracle in ur ownership ...
>
> "u" and "ur" are not in my dictionary, perhaps you mean "you" and "your"
> ?

are there any internals coding standards somewhere?

moshe.

Derick Rethans

23 years ago

On Thu, 14 Aug 2003, Steven Brown wrote:

> E.g.,
>
> $id = "0; drop bar";
> lamesql_query("select foo from bar where id = $id");

Well Do'h, how is this a security problem?

Derick

--
"Interpreting what the GPL actually means is a job best left to those
that read the future by examining animal entrails."
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
International PHP Magazine                          http://php-mag.net/
-------------------------------------------------------------------------

Mark J. Hershenson

23 years ago

On Thursday, Aug 14, 2003, at 00:45 US/Pacific, Steven Brown wrote:

>>
>> On Thu, 14 Aug 2003, moshe doron wrote:
>>
>>> What about hacking somehow the sqlite library to disallow chained
>>> queries (or at least do it optionally)?
>>>
>>> This behavior is *huge* security hole, allow to the cracker drop ur
>>> database using simple select where query.
>>
>> How is this a security hole?
>
> E.g.,
>
> $id = "0; drop bar";
> lamesql_query("select foo from bar where id = $id");

I've read this entire thread, and while I think the thread is well
taken in toto, I'm wondering why it is that:

$id = intval( "0; drop bar" );

if( $id > 0 )
{
lamesql_query( some_escape_string_function( "select foo from bar
where id = '$id'" ) );
}
else
{
// Error?
}

Is so much harder to program or remember to do - it's just second
nature once you get in the habit of doing it.

Failing to account for one of the the core cardinal rules that you
never ever ever trust user input as a programmer is just bad
programming, not the fault of the language you do it in, just as it
wouldn't be the fault of the text editor you used or the computer you
were working on.

Casting and escape_string()'ing and quoting your field values are just
some of the good practices when working with a database or dropping out
to a shell. However you go about it, it's imperative to understand your
underlying data, datasource, and exactly how the data gets sent to and
from your datasource.

The security hole described herein lies almost entirely on the
programmer...and probably should.

--  mjh

Mark J. Hershenson

23 years ago

Sorry:

That code should have been:

$id = some_escape_string_function( intval( "0; drop bar" ) );

if( $id > 0 )
{
lamesql_query( "select foo from bar where id = '$id'" ) );
}
else
{
// Error?
}

Just wanted to be precise in what I was trying to convey.

--  mjh

Ard Biesheuvel

23 years ago

> What about hacking somehow the sqlite library to disallow chained

queries

> (or at least do it optionally)?

If you are unable or unwilling to verify the safety of your input,
use a database that supports params + binding (like Firebird :-))

Any input obtained from the client should be considered unsafe
(== tainted in Perl) and should be checked for correctness first.
In the described case, casting the value to a number if it's
expected to be numerical or putting quotes around it if it's expected
to be a string value will solve your problem. Disallowing chained
queries will prevent *every* developer from ever using it.

Ard

Hartmut Holzgraefe

23 years ago

Ard Biesheuvel wrote:

> Disallowing chained
> queries will prevent *every* developer from ever using it.

thats why i suggested earlier to maybe make it a runtime
configurable feature that defaults to 'off'

--
Hartmut Holzgraefe  <hartmut@php.net>

Ilia A.

23 years ago

First of all this discussion bares to relevance to the 4.3.3 release as sqlite
is NOT part of this release. Secondly this is just plain silly. PHP is not
and is not responsible for validating input. If the user chooses not to and
consequently leaves their scripts vulnreable to SQL injection it is their
fault and their fault alone.
Ability to chain queries is an extremely useful feature that most database
systems support (even MySQL as of version 4.0). To cripple or disable such
functionality would be absolute idiocy not to mention break backwards
compatibility to older versions where this was possible. Adding more run-time
directives (as suggested by Hartmut Holzgraefe ) is a bad idea as it makes
writing portable code extremely difficult as each system may have a
drastically different behavior due to an ini option.

Ilia

Rasmus Lerdorf

23 years ago

Right, and in the end this should be done on a per-site basis through the
input filtering mechanism I added to PHP5 a while ago.

-Rasmus

On Thu, 14 Aug 2003, Ilia Alshanetsky wrote:

Steven Brown

23 years ago

(Btw, sorry for leaving ilia off this, but ilia@prohost.org isn't
resolving and the mail server here is bugged atm and refusing to accept
the message)

> -----Original Message-----
> From: Ilia Alshanetsky [mailto:ilia@prohost.org]
> Sent: Thursday, August 14, 2003 8:07 AM
> To: moshe doron; internals@lists.php.net
> Subject: Re: [PHP-DEV] Re: PHP 4.3.3RC3 Released

> First of all this discussion bares to relevance to the 4.3.3
> release as sqlite
> is NOT part of this release. Secondly this is just plain
> silly. PHP is not
> and is not responsible for validating input. If the user
> chooses not to and
> consequently leaves their scripts vulnreable to SQL injection
> it is their
> fault and their fault alone.

It's insanely easy to make such mistakes though, and php.net is full of
such problems (I found 2 in 15 minutes), so it's not a 'stupid
developer' issue, as you guys make the same mistakes, and are extremely
experienced.  Why make such bugs massive security holes when with a good
design in PHP itself, you can reduce them to often just minor errors, or
even prevent their class 100%?  I.e., you can do without snprintf and
only use sprintf, but there's a reason for snprintf to exist, don't you
agree?

The problem is string compositing SQL queries.  With a SQL command
builder interface, query hacks don't occur.  Right now, string
compositing mysql queries with unvalidated input often results in just
minor bugs, as from what I hear the mysql extention can't do chained
queries yet, so there's no risk of someone injecting whatever SQL they
choose, making it much harder to do evil.

> Ability to chain queries is an extremely useful feature that
> most database
> systems support (even MySQL as of version 4.0). To cripple or
> disable such
> functionality would be absolute idiocy not to mention break backwards
> compatibility to older versions where this was possible.

If you want to allow chained queries, please consider only allowing them
through a SQL command builder interface.  They are just too dangerous to
allow string composited queries to use.  If you really care about
security, deprecate string composited queries altogether in favor of a
builder interface.  That would eliminate a huge class of PHP hacks, and
the need for the magic quotes system.

Why not?

John Coggeshall

23 years ago

> It's insanely easy to make such mistakes though, and php.net is full of
> such problems (I found 2 in 15 minutes), so it's not a 'stupid

I'd like to hear where these problems are, off list of course.

> minor bugs, as from what I hear the mysql extention can't do chained
> queries yet, so there's no risk of someone injecting whatever SQL they
> choose, making it much harder to do evil.

The current ext/mysql extension doesn't allow multiple queries...
however, it is by no means idiot-proof.. consider:

UPDATE auth_user set password='{$_POST['newpass']}' WHERE
username='$uname';

Now let's say 'newpass' is the string "foo' WHERE username='admin' --"

That'd convert the query to:

UPDATE auth_user set password='foo' WHERE username='admin' -- WHERE
username='realusername'

As you can see, even in mysql it is very easy to do evil despite your
claims. IMO and I believe the opinion of most of the internals community
it is not the responsibility of PHP to correct these and other potential
security holes.

Please let this issue drop.

John

--
-~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~-
John Coggeshall                             http://www.coggeshall.org/
john at coggeshall dot org

The PHP Developer's Handbook
The definitive PHP5 developer's guide     http://www.php-handbook.com/
-~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~--~=~-

Steven Brown

23 years ago

> -----Original Message-----
> From: John Coggeshall [mailto:john@coggeshall.org]
> Sent: Thursday, August 14, 2003 10:17 PM
> To: Steven Brown
> Cc: 'moshe doron'; PHP Internals
> Subject: RE: [PHP-DEV] Re: PHP 4.3.3RC3 Released
>
>
> > It's insanely easy to make such mistakes though, and
> php.net is full
> > of such problems (I found 2 in 15 minutes), so it's not a 'stupid
>
> I'd like to hear where these problems are, off list of course.

I already posted one example in this thread.

> UPDATE auth_user set password='foo' WHERE username='admin' --
> WHERE username='realusername'
>
> As you can see, even in mysql it is very easy to do evil
> despite your claims.

But you had to do that on an UPDATE query, and you couldn't inject
arbitrary SQL.  Compare that to the alternative of string-built chained
queries, where you can do whatever you want 100% of the time.  That's
the point, not to make every mistake a guaranteed 100% full exploit by
properly designing the interface, e.g., requiring a SQL command builder
interface.  Same reason UNIX has more than just 'root'; you might hack
'games', but it might not lead to a full compromise.

> IMO and I believe the opinion of most of
> the internals community it is not the responsibility of PHP
> to correct these and other potential security holes.
>
> Please let this issue drop.

I'll let it drop, but I think your views will result in dangerous
interfaces and I hope you reconsider.  If you enable chained queries in
the mysql extension as-is for example, it will enable a massive flood of
hacking.

Kevin Waterson

23 years ago

This one time, at band camp, "Steven Brown" <swbrown@ucsd.edu> wrote:

> I'll let it drop, but I think your views will result in dangerous
> interfaces and I hope you reconsider.  If you enable chained queries in
> the mysql extension as-is for example, it will enable a massive flood of
> hacking.

Please, please, please, please, please do not remove chained queries.
I like them. I want them. I know how to validate user input.
if you don't like them, kill and query with a semi-colon in it.

Kind regards
Kevin

--
______
(_____ \
_____) )  ____   ____   ____   ____
|  ____/  / _  ) / _  | / ___) / _  )
| |      ( (/ / ( ( | |( (___ ( (/ /
|_|       \____) \_||_| \____) \____)
Kevin Waterson
Port Macquarie, Australia

Rasmus Lerdorf

23 years ago

On Thu, 14 Aug 2003, Steven Brown wrote:

> It's insanely easy to make such mistakes though, and php.net is full of
> such problems (I found 2 in 15 minutes), so it's not a 'stupid
> developer' issue, as you guys make the same mistakes, and are extremely
> experienced.

That's interesting considering we are using MySQL and the mysql extension
code does not allow multiple queries.  So I don't see how you could have
found such an exploit.

-Rasmus

Steven Brown

23 years ago

> -----Original Message-----
> From: Rasmus Lerdorf [mailto:rasmus@lerdorf.com]
> Sent: Thursday, August 14, 2003 10:37 PM
> To: Steven Brown
> Cc: internals@lists.php.net
> Subject: RE: [PHP-DEV] Re: PHP 4.3.3RC3 Released
>
>
> On Thu, 14 Aug 2003, Steven Brown wrote:
> > It's insanely easy to make such mistakes though, and
> php.net is full
> > of such problems (I found 2 in 15 minutes), so it's not a 'stupid
> > developer' issue, as you guys make the same mistakes, and are
> > extremely experienced.
>
> That's interesting considering we are using MySQL and the
> mysql extension code does not allow multiple queries.  So I
> don't see how you could have found such an exploit.

I found an example of passing unvalidated input into a SQL query, I
didn't say it would lead to an exploit.  The point was, yes, even you
guys make this mistake.  It's not a "Well, you should have learned to
write secure code" type of issue.  Everyone makes this mistake
occasionally.

Rasmus Lerdorf

23 years ago

On Thu, 14 Aug 2003, Steven Brown wrote:

> I found an example of passing unvalidated input into a SQL query, I
> didn't say it would lead to an exploit.  The point was, yes, even you
> guys make this mistake.  It's not a "Well, you should have learned to
> write secure code" type of issue.  Everyone makes this mistake
> occasionally.

Unvalidated in what sense?  We don't validate for multiple queries because
we have no need to do so.

-Rasmus

Steven Brown

23 years ago

> -----Original Message-----
> From: Rasmus Lerdorf [mailto:rasmus@lerdorf.com]
> Sent: Thursday, August 14, 2003 10:53 PM
> To: Steven Brown
> Cc: internals@lists.php.net
> Subject: RE: [PHP-DEV] Re: PHP 4.3.3RC3 Released
>
>
> On Thu, 14 Aug 2003, Steven Brown wrote:
> > I found an example of passing unvalidated input into a SQL query, I
> > didn't say it would lead to an exploit.  The point was,
> yes, even you
> > guys make this mistake.  It's not a "Well, you should have
> learned to
> > write secure code" type of issue.  Everyone makes this mistake
> > occasionally.
>
> Unvalidated in what sense?  We don't validate for multiple
> queries because we have no need to do so.

Unvalidated in that I could modify the SQL query as my text in the URL
made it into the query unvalidated, unquoted, and unescaped.

To recap the thread, the argument was originally between moshe and the
list that sqlite's chained queries (I assume something like "select *
from foo; drop foo") were a security hazard and should be disabled due
to it upgrading all SQL command validation bugs (possible database
compromise bugs, e.g., modification of certain UPDATEs) to guaranteed
database compromise bugs.  The list prestented the argument that any
validation bug is the sign of a bad programmer not validating user input
so this was a non-issue:

(Derick):
Well, that's not our problem if people don't verify user input. Every
article dealing with the simplest security problem mentions this. I
don't think it's a good idea to break this functionality. (yes, even
MySQL has this in version 4.1+)

(Derick):
But it's a *user* problem (the developer), not a PHP problem. PHP
should
not break nice functionality in an extension (such as chaining
queries)
because of people too lazy to verify user input.

I jumped in at this point and pointed out all developers occasionally
have validation mistakes in their code, php.net included (see my earlier
post with an example query against php.net).  It's the wrong mindset to
think "any bug should be a full compromise bug, as it's the programmer's
fault".

My proposed solution was to only allow something as dangerous as chained
queries to be used through a SQL command builder interface, and ideally
to deprecate the query functions that accept query strings.  There are
too many PHP hacks happening already due to SQL injections.  There's no
reason for this to be a class of bug at all.  A command builder totally
eliminates these kind of bugs.

Rasmus Lerdorf

23 years ago

On Thu, 14 Aug 2003, Steven Brown wrote:

> > -----Original Message-----
> > From: Rasmus Lerdorf [mailto:rasmus@lerdorf.com]
> > Sent: Thursday, August 14, 2003 10:53 PM
> > To: Steven Brown
> > Cc: internals@lists.php.net
> > Subject: RE: [PHP-DEV] Re: PHP 4.3.3RC3 Released
> >
> >
> > On Thu, 14 Aug 2003, Steven Brown wrote:
> > > I found an example of passing unvalidated input into a SQL query, I
> > > didn't say it would lead to an exploit.  The point was,
> > yes, even you
> > > guys make this mistake.  It's not a "Well, you should have
> > learned to
> > > write secure code" type of issue.  Everyone makes this mistake
> > > occasionally.
> >
> > Unvalidated in what sense?  We don't validate for multiple
> > queries because we have no need to do so.
>
> Unvalidated in that I could modify the SQL query as my text in the URL
> made it into the query unvalidated, unquoted, and unescaped.

So?  There is no possibility of an exploit here, so the validation is
adequate.

And I have no idea why you copied the entire thread back to me.  I read
it.  We cannot possibly predict what user input will be problematic in the
various backends.  The query chaining char, if supported, is likely
different from one backend to the next, and chars that may be illegal in
other ways vary as well.  On top of that, specific application
characteristics will introduce another set of dangerous data.  People need
to move towards policy-based input filtering and not rely on what can only
be pathetically incomplete higher level filters.

-Rasmus

Steven Brown

23 years ago

> -----Original Message-----
> From: Rasmus Lerdorf [mailto:rasmus@lerdorf.com]
> Sent: Thursday, August 14, 2003 11:15 PM
> To: Steven Brown
> Cc: internals@lists.php.net
> Subject: RE: [PHP-DEV] Re: PHP 4.3.3RC3 Released

> On Thu, 14 Aug 2003, Steven Brown wrote:
> > > -----Original Message-----
> > > From: Rasmus Lerdorf [mailto:rasmus@lerdorf.com]
> > > Sent: Thursday, August 14, 2003 10:53 PM
> > > To: Steven Brown
> > > Cc: internals@lists.php.net
> > > Subject: RE: [PHP-DEV] Re: PHP 4.3.3RC3 Released

> > > Unvalidated in what sense?  We don't validate for
> multiple queries
> > > because we have no need to do so.
> >
> > Unvalidated in that I could modify the SQL query as my text
> in the URL
> > made it into the query unvalidated, unquoted, and unescaped.
>
> So?  There is no possibility of an exploit here, so the
> validation is adequate.

The point was that even excellent programmers fail to validate the stuff
that goes into SQL queries sometimes, not to hack php.net.

Btw, it's also never adequate validation to let users inject their own
SQL, even in a SELECT.  They might add impossibly complex stuff to the
where clause and cause your database to wedge for days, or add a 'or 1 =
1' to bypass security checks for information.

However, it's relatively difficult to turn any arbitrary query into
something really nasty.  If you could chain queries without any safer
PHP-provided interface, it would be trivial to inject whatever SQL you
wanted via these common bugs.  That's the other point.

> And I have no idea why you copied the entire thread back to
> me.  I read it.

Because I figured you wouldn't be pointing out that my php.net example
isn't an exploit if you had read it.

> We cannot possibly predict what user input
> will be problematic in the various backends.  The query
> chaining char, if supported, is likely different from one
> backend to the next, and chars that may be illegal in other
> ways vary as well.  On top of that, specific application
> characteristics will introduce another set of dangerous data.

Yes, which is why a command builder povided by the backend wrapper is a
much safer solution than passing strings.  To me, the desire to add
chained queries to a backend forces the issue of developing and
promoting such interfaces, as otherwise, there's going to be a lot of
seriously insecure code out there all of a sudden.

Why not have a builder for sqlite if you plan on keeping its chaining
functionality?  What's the desire to make it easier for folks to create
big security holes instead of small ones - that no one wants to write
the builder?

Derick Rethans

23 years ago

On Thu, 14 Aug 2003, Rasmus Lerdorf wrote:

> On Thu, 14 Aug 2003, Steven Brown wrote:
> > I found an example of passing unvalidated input into a SQL query, I
> > didn't say it would lead to an exploit.  The point was, yes, even you
> > guys make this mistake.  It's not a "Well, you should have learned to
> > write secure code" type of issue.  Everyone makes this mistake
> > occasionally.
>
> Unvalidated in what sense?  We don't validate for multiple queries because
> we have no need to do so.

That one is already fixed too btw, see my last commit to bugs-web.

Derick

--
"Interpreting what the GPL actually means is a job best left to those
that read the future by examining animal entrails."
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
International PHP Magazine                          http://php-mag.net/
-------------------------------------------------------------------------

Ard Biesheuvel

23 years ago

> The problem is string compositing SQL queries.  With a SQL command
> builder interface, query hacks don't occur.  Right now, string
> compositing mysql queries with unvalidated input often results in just
> minor bugs, as from what I hear the mysql extention can't do chained
> queries yet, so there's no risk of someone injecting whatever SQL they
> choose, making it much harder to do evil.

Instead of

'SELECT .. WHERE id=$id'

write

'SELECT ... WHERE id='. (int)$id

Instead of

'SELECT .. WHERE name=$name'

write

'SELECT ... WHERE name=" '.addslashes($name).' " '

Problem solved forever

Ard

Jedi/Sector One

23 years ago

On Fri, Aug 15, 2003 at 11:19:47AM +0200, Ard Biesheuvel wrote:

> 'SELECT ... WHERE id='. (int)$id

Even shorter :

'SELECT ... WHERE id='. $id+0

> 'SELECT ... WHERE name=" '.addslashes($name).' " '

Wasn't addslashes() designed to escape shell commands and
mysql_escape_string() for MySQL queries?

--
__  /*-      Frank DENIS (Jedi/Sector One) <j@42-Networks.Com>     -*\  __
\ '/    <a href="http://www.PureFTPd.Org/"> Secure FTP Server </a>    \' /
\/  <a href="http://www.Jedi.Claranet.Fr/"> Misc. free software </a>  \/

Ard Biesheuvel

23 years ago

>   Wasn't addslashes() designed to escape shell commands and
> mysql_escape_string() for MySQL queries?

That's right.

Ard