Everything

php

open_basedir enhancement, runtime tightening

php.internals

Sara Golemon

19 years ago

The attached patch changes open_basedir from PHP_INI_SYSTEM to PHP_INI_ALL.

Wait now... let me finish.

It introduces a custom INI modification handler for open_basedir which
allows the option to be set during Startup and Shutdown (PHP_INI_SYSTEM
contexts) normally, then for other contexts (Activate/Deactivate {PERDIR}
and Runtime) it checks the inbound settings against the current value and
applies some logic:

If open_basedir hasn't been set yet:  It allows any new setting to be
applied (unrestrictive to restrictive)

If open_basedir is set, then it checks to see if each component of the new
setting would be allowable under the rules of the old setting,  if they're
all good, it allows it.  If any component makes the basedir check less
restrictive, it fails (diallows) the change. (less-restrictive to
more-restrictive).

The advantage of doing this is that package authors and/or users of shared
hosting who may not have access to making their settings more restrictive
can avoid most simple FS inspection attacks caused by buggy script code by
adding a single ini_set(basedir(__FILE__));  to the top of their script or
setting it with an .htaccess directive.

Note that it doesn't do a thing to avoid code inejction attacks as such an
attacker could issue ini_restore("open_basedir"); and have the same access
to the FS as they would without this patch.  I'll grant you it's not a
panacea, and it may be more harmful than good by making people think that
tightening up open_basedir is enough, but it's something.

Talk amongst y'selves....

-Sara

Andrei Zmievski

19 years ago

I don't have a problem with this.

-Andrei

On Oct 12, 2006, at 2:53 PM, Sara Golemon wrote:

Richard Lynch

19 years ago

On Thu, October 12, 2006 2:53 pm, Sara Golemon wrote:

> The attached patch changes open_basedir from PHP_INI_SYSTEM to
> PHP_INI_ALL.

My only "beef" would be if the docs claimed it was just plain old
PHP_INI_ALL without explaining the custom restrictions...

It will cause all manner of confusion that it behaves uniquely unless
it's documented.

+1 with documentation
-1 without documentation

--
Some people have a "gift" link here.
Know what I want?
I want you to buy a CD from some starving artist.
http://cdbaby.com/browse/from/lynch
Yeah, I get a buck. So?

Tim Starling

19 years ago

Sara Golemon wrote:

> The attached patch changes open_basedir from PHP_INI_SYSTEM to PHP_INI_ALL.

[...]

> The advantage of doing this is that package authors and/or users of shared
> hosting who may not have access to making their settings more restrictive
> can avoid most simple FS inspection attacks caused by buggy script code by
> adding a single ini_set(basedir(__FILE__));  to the top of their script or
> setting it with an .htaccess directive.

Great feature. I can see this being very useful to packaged PHP applications
like ours (MediaWiki). The only complication in implementation I can think
of is trying to work out the location of PEAR, for those modules that use
it. I suppose we would have to append the default include_path to the
runtime open_basedir, to make sure that PEAR is accessible.

-- Tim Starling

Sander Steffann

19 years ago

Hi,

> Great feature. I can see this being very useful to packaged
> PHP applications
> like ours (MediaWiki). The only complication in
> implementation I can think
> of is trying to work out the location of PEAR, for those
> modules that use
> it. I suppose we would have to append the default include_path to the
> runtime open_basedir, to make sure that PEAR is accessible.

Be careful here. The things in the include_path should only be added to the
open_basedir setting if they were already present in the original
open_basedir.

- Sander

Pierre Joye

19 years ago

Hello,

On 10/13/06, Tim Starling <tstarling@wikimedia.org> wrote:

> Sara Golemon wrote:
> > The attached patch changes open_basedir from PHP_INI_SYSTEM to PHP_INI_ALL.
> [...]
>
> > The advantage of doing this is that package authors and/or users of shared
> > hosting who may not have access to making their settings more restrictive
> > can avoid most simple FS inspection attacks caused by buggy script code by
> > adding a single ini_set(basedir(__FILE__));  to the top of their script or
> > setting it with an .htaccess directive.
>
> Great feature. I can see this being very useful to packaged PHP applications
> like ours (MediaWiki). The only complication in implementation I can think
> of is trying to work out the location of PEAR, for those modules that use
> it. I suppose we would have to append the default include_path to the
> runtime open_basedir, to make sure that PEAR is accessible.

There is no issue with PEAR or any applications using include_path and
relative paths in include/require. The system include_path, if any,
paths should already  be in the open_basedir. If they are not, you
have to install the desired modules within your open_basedir, just
like now.

Sara, I did not check the patch (not readable here :P), but I like the idea.

--Pierre

Tim Starling

19 years ago

Pierre wrote:

> There is no issue with PEAR or any applications using include_path and
> relative paths in include/require. The system include_path, if any,
> paths should already  be in the open_basedir. If they are not, you
> have to install the desired modules within your open_basedir, just
> like now.

The application I'm interested in is where there is no system open_basedir,
and the application wishes to lock down the environment. So we could do:

ini_set('open_basedir', ini_get('include_path') . PATH_SEPARATOR .
dirname(__FILE__));

But that still allows the application to access every path that the system
administrator has, at a whim, included in include_path. It's not maximally
restrictive, really we only need dirname(__FILE__) and wherever PEAR is.

We could do:
$pear = trim(`pear config-get php_dir`);
ini_set('open_basedir', $pear . PATH_SEPARATOR . dirname(__FILE__);

But of course, that's not very portable. What I'm hinting at is that it
might be kind of nice if PHP knew where PEAR was, and provided it say via
$_SERVER. It might save a bit of mucking around.

System administrators are usually pretty clueless about PHP compared to
application developers. It's up to the application to make sense of whatever
insecure, inconsistent mess the sysadmin has left the system in, and to run
in that environment as securely as possible. That's why I welcome a feature
such as this.

Now if there was just some way for the application to control
magic_quotes_gpc and register_globals, I'd be a happy man.

-- Tim Starling

Greg Beaver

19 years ago

Tim Starling wrote:

> Pierre wrote:
>> There is no issue with PEAR or any applications using include_path and
>> relative paths in include/require. The system include_path, if any,
>> paths should already  be in the open_basedir. If they are not, you
>> have to install the desired modules within your open_basedir, just
>> like now.
>
> The application I'm interested in is where there is no system
> open_basedir, and the application wishes to lock down the environment.
> So we could do:
>
> ini_set('open_basedir', ini_get('include_path') . PATH_SEPARATOR .
> dirname(__FILE__));
>
> But that still allows the application to access every path that the
> system administrator has, at a whim, included in include_path. It's not
> maximally restrictive, really we only need dirname(__FILE__) and
> wherever PEAR is.
>
> We could do:
> $pear = trim(`pear config-get php_dir`);
> ini_set('open_basedir', $pear . PATH_SEPARATOR . dirname(__FILE__);
>
> But of course, that's not very portable. What I'm hinting at is that it
> might be kind of nice if PHP knew where PEAR was, and provided it say
> via $_SERVER. It might save a bit of mucking around.

Hi Tim,

This is a bit more PEAR-related, please follow up to pear-dev with any
further questions.  The code you're looking for is:

require_once 'PEAR/Config.php';
$c = PEAR_Config::singleton(); // assuming PHP 5, add & if PHP 4
ini_set('open_basedir', $c->get('php_dir') . PATH_SEPARATOR .
dirname(__FILE__));

Greg

Pierre Joye

19 years ago

Hello,

On 10/13/06, Gregory Beaver <greg@chiaraquartet.net> wrote:

> Tim Starling wrote:
> > Pierre wrote:
> >> There is no issue with PEAR or any applications using include_path and
> >> relative paths in include/require. The system include_path, if any,
> >> paths should already  be in the open_basedir. If they are not, you
> >> have to install the desired modules within your open_basedir, just
> >> like now.
> >
> > The application I'm interested in is where there is no system
> > open_basedir, and the application wishes to lock down the environment.
> > So we could do:
> >
> > ini_set('open_basedir', ini_get('include_path') . PATH_SEPARATOR .
> > dirname(__FILE__));
> >
> > But that still allows the application to access every path that the
> > system administrator has, at a whim, included in include_path. It's not
> > maximally restrictive, really we only need dirname(__FILE__) and
> > wherever PEAR is.
> >
> > We could do:
> > $pear = trim(`pear config-get php_dir`);
> > ini_set('open_basedir', $pear . PATH_SEPARATOR . dirname(__FILE__);
> >
> > But of course, that's not very portable. What I'm hinting at is that it
> > might be kind of nice if PHP knew where PEAR was, and provided it say
> > via $_SERVER. It might save a bit of mucking around.
>
> Hi Tim,
>
> This is a bit more PEAR-related, please follow up to pear-dev with any
> further questions.  The code you're looking for is:
>
> require_once 'PEAR/Config.php';
> $c = PEAR_Config::singleton(); // assuming PHP 5, add & if PHP 4
> ini_set('open_basedir', $c->get('php_dir') . PATH_SEPARATOR .
> dirname(__FILE__));

These values do not change during the application lifetime. Dynamic
tests and decisions are ugly when what you test never change. I would
suggest to do it only during the installation stage.

--Pierre