Nuking safe_mode - Externals

19 years ago by Andi Gutmans — view source — reply

unread

Hey,

I'm nuking safe_mode and I found something odd. In streams,
php_plain_files_unlink() only checks php_check_open_basedir() when
ENFORCE_SAFE_MODE is flagged. I was planning on nuking
ENFORCE_SAFE_MODE completely. Is this a bug? Or should I rename
ENFORCE_SAFE_MODE to ENFORCE_OPEN_BASEDIR?

Andi

19 years ago by Andi Gutmans — view source — reply

unread

I just saw we have STREAM_DISABLE_OPEN_BASEDIR.
Should I just check if that is 0 and do the open_basedir check in that case?

At 05:02 PM 2/18/2006, Andi Gutmans wrote:

Hey,

I'm nuking safe_mode and I found something odd. In streams,
php_plain_files_unlink() only checks php_check_open_basedir() when
ENFORCE_SAFE_MODE is flagged. I was planning on nuking
ENFORCE_SAFE_MODE completely. Is this a bug? Or should I rename
ENFORCE_SAFE_MODE to ENFORCE_OPEN_BASEDIR?

Andi

19 years ago by Sara Golemon — view source — reply

unread

I'm nuking safe_mode and I found something odd. In streams,
php_plain_files_unlink() only checks php_check_open_basedir() when
ENFORCE_SAFE_MODE is flagged. I was planning on nuking ENFORCE_SAFE_MODE
completely. Is this a bug? Or should I rename ENFORCE_SAFE_MODE to
ENFORCE_OPEN_BASEDIR?

I just saw we have STREAM_DISABLE_OPEN_BASEDIR.
Should I just check if that is 0 and do the open_basedir check in that
case?

Agreed. This slight logic twist got intrduced when unlink was routed
through wrapper->ops. It should be:

if (ENFORCE_SAFE_MODE) { /* safe mode check*/ }
if (!DISABLE_OPEN_BASEDIR) { /* do open basedir check */ }

Bad news, I'm probably the one who did that, good news the practical fact of
the matter is that it'd be uncommon to use both or neither option. It's
more "normal" to use one or the other so the logic as-is isn't particularly
harmful, just.... not-entirely-right. Looking at a quick grep, the only use
of wops->unlink() I actually see is the one in
ext/standard/file.c:PHP_FUNCTION(unlink) anyway.

-Sara

19 years ago by Andi Gutmans — view source — reply

unread

Thanks Sara. I'll fix accordingly. Please watch my commits on the
wrappers and make sure I'm doing the right thing.

At 07:40 PM 2/18/2006, Sara Golemon wrote:

I'm nuking safe_mode and I found something odd. In streams,
php_plain_files_unlink() only checks php_check_open_basedir() when
ENFORCE_SAFE_MODE is flagged. I was planning on nuking
ENFORCE_SAFE_MODE completely. Is this a bug? Or should I rename
ENFORCE_SAFE_MODE to ENFORCE_OPEN_BASEDIR?
I just saw we have STREAM_DISABLE_OPEN_BASEDIR.
Should I just check if that is 0 and do the open_basedir check in that case?
Agreed. This slight logic twist got intrduced when unlink was
routed through wrapper->ops. It should be:

if (ENFORCE_SAFE_MODE) { /* safe mode check*/ }
if (!DISABLE_OPEN_BASEDIR) { /* do open basedir check */ }

Bad news, I'm probably the one who did that, good news the practical
fact of the matter is that it'd be uncommon to use both or neither
option. It's more "normal" to use one or the other so the logic
as-is isn't particularly harmful, just....
not-entirely-right. Looking at a quick grep, the only use of
wops->unlink() I actually see is the one in
ext/standard/file.c:PHP_FUNCTION(unlink) anyway.

-Sara

19 years ago by Peter Brodersen — view source — reply

unread

Hi,

Just got home from a month in South America and is trying to catch up
on old posts...

On Sat, 18 Feb 2006 17:02:32 -0800, in php.internals andi@zend.com
(Andi Gutmans) wrote:

I'm nuking safe_mode and I found something odd. In streams,
php_plain_files_unlink() only checks php_check_open_basedir() when
ENFORCE_SAFE_MODE is flagged. I was planning on nuking
ENFORCE_SAFE_MODE completely. Is this a bug? Or should I rename
ENFORCE_SAFE_MODE to ENFORCE_OPEN_BASEDIR?

Rasmus mentions in 43874C56.8050007@lerdorf.com /
http://news.php.net/php.internals/20417 that it might be a good idea
unwrapping the safe_mode_exec_dir check from the uid matching.

The trouble about "safe mode" seems mainly to be the expression (based
on the notes in
http://www.php.net/~derick/meeting-notes.html#safe-mode ) and the
spurious UID checks.

In the same old thread it didn't seem like there was a consensus or
best practice regarding the exec functions. Some recommended using
disabled_functions, but this would just be asking people to "remake"
and maintain their own list of functions that safe_mode (under any
other name) would have disabled for them.

Would the feature of safe_mode_exec_dir in any kind of name be
preserved, as recommended by Rasmus? This might be the exact time to
"remind us later".

Peter Brodersen