[log4perl-devel] For consideration: patch to use Safe compartments in Log::Log4per l::Config

[James.FitzGibbon <Jam...@ta...>]

Attached is a patch that implements the change I describe in my message to
Mike
Schilli earlier today.  The patch includes code changes, new POD and six new
test cases.

The issues that I still have to address after getting some peer feedback:

- the use of a Safe compartment is not enforced for the user-defined cspecs
in
Log::Log4perl::Layout::PatternLayout (though the POD changes suggest that
they
are).

I do intend to bring the Safe compartment to PatternLayout, but I notice
that the existing code has it's own eval statement rather than borrowing
from
Log::Log4perl::Config::compile_if_perl().  Rather than re-implement the
changes
I made to compile_if_perl() in PatternLayout.pm, I wanted to check if there
is
any reason why they couldn't both use the same routine.

- My original message suggested that there would only be a single new
variable,
but in implementing it I found that I actually needed three extra.  This
makes
it look a bit ugly.  It's the exporting of symbols from other packages into
the
compartment that is the biggest eyesore IMHO.  The amount of information in
the
package globals really makes me feel like this information needs to go
somewhere
else, but I don't know where.  I'm open to suggestions.

- The POD explaining how the Safe compartment restrictions work is in
Log/Log4perl.pm
right now, and is a bit verbose.  A quicker intro could be made for this
module and
the detailed verbage moved to Log/Log4perl/Config.pm instead.

Please review and let me know what needs touching up before this could
become
part of the regular Log::Log4perl distribution.

Thanks.

--
j.

James FitzGibbon                    
Consultant, Ajilon Services, TTS-3D@CC-950 
jam...@ta...
voice/fax 612-304-6121/3277




-----Original Message-----
From: James.FitzGibbon 
Sent: Thursday, May 01, 2003 10:48 AM
To: 'm...@pe...'
Subject: Open to use of Safe in Log::Log4perl::Config?


Hi there.

I'm playing with Log4perl for the first time, and while I'm very impressed
with the
feature set, the ability to execute completely arbitrary code worries me.  I
have
a need to execute a limited number of Perl ops, but I don't want people
using
destructive operators.

Would you be open to a patch to Log::Log4perl::Config that uses the Safe
Module to
restrict the operations that can be performed within compile_if_perl()?

I'm envisioning something like this in a program:

$Log::Log4perl::ALLOWED_CODE_OPS_IN_CONFIG_FILE = ':browse';
Log::Log4perl->init( {
	log.log4perl.logger => 'ERROR, Main',
	log.log4perl.appender.Main = Log::Dispatch::File,
	log.log4perl.appender.Main.filename = sub { "$ENV{LOGNAME}.log" },
} );

Which would pass, but:

Log::Log4perl->init( {
	log.log4perl.logger => 'ERROR, Main',
	log.log4perl.appender.Main = Log::Dispatch::File,
	log.log4perl.appender.Main.filename = sub { system("rm -rf /" ) },
} );

Would throw an error like "operation 'system' trapped by mask ':browse'".

For compatibility, having $Log::Log4perl::ALLOW_CODE_IN_CONFIG_FILE set to
'1'
would not enforce any restrictions.  For ease of setup without people having
to know how Safe works, I was thinking that if
$Log::Log4perl::ALLOW_CODE_IN_CONFIG_FILE was set to 'safe' then it would be
mapped to ':browse', which would allow for most read operations, including
things like 'gethostbyname'.  This mapping could be extended so that there
could
be other things like 'restricted' that just equaled ':default'.

Your thoughts?

Thanks.

--
j.

James FitzGibbon                    
Consultant, Ajilon Services, TTS-3D@CC-950 
jam...@ta...
voice/fax 612-304-6121/3277