Re: File::Temp: euid in _is_safe(); safe_level(MEDIUM) for taint mode

2005-09-02 Thread Rafael Garcia-Suarez 
Alexey Tourbin wrote:
 Hello,
 
 The patch is explained below.

This patch looks sensible to me. Tim, any objection ?

Also, why use ${\cTAINT} instead of ${^TAINT} ? for older perls I suppose ?

 --- File/Temp.pm- 2005-04-03 15:27:16 +
 +++ File/Temp.pm  2005-08-16 22:50:39 +
 @@ -679,11 +679,11 @@ sub _is_safe {
return 1 if $^O eq 'VMS';  # owner delete control at file level
  
# Check to see whether owner is neither superuser (or a system uid) nor me
 -  # Use the real uid from the $ variable
 +  # Use the effective uid from the $ variable
# UID is in [4]
 -  if ($info[4]  File::Temp-top_system_uid()  $info[4] != $) {
 +  if ($info[4]  File::Temp-top_system_uid()  $info[4] != $) {
  
 -Carp::cluck(sprintf uid=$info[4] topuid=%s \$=$ path='$path',
 +Carp::cluck(sprintf st_uid=$info[4] topuid=%s euid=$ path='$path',
   File::Temp-top_system_uid());
  
  $$err_ref = Directory owned neither by root nor the current user
 @@ -2241,4 +2241,10 @@ security enhancements.
  
  =cut
  
 +{
 +no strict 'refs';
 +File::Temp-safe_level(MEDIUM)
 +if ${\cTAINT};
 +}
 +
  1;
 End of patch
 
 First, real/effective UID distinction is essential for setuid scripts.
 Filesystem permissions are controlled by the effective UID of the
 process.  When a privileged script is checking if the directory is safe,
 it should check that the directory is *not* owned by the caller.
 Otherwise, the user can replace a temporary file created by the
 privileged process, which is almost certainly not what we want.
 
 Second, I suggest to enable MEDUM security level for taint mode,
 which is on when running setuid/setgid scripts.  It's only on MEDUM
 level that the above _is_safe() security check is performed.

Re: File::Temp: euid in _is_safe(); safe_level(MEDIUM) for taint mode

2005-09-02 Thread Tim Jenness 

On Fri, 2 Sep 2005, Rafael Garcia-Suarez wrote:


Alexey Tourbin wrote:

Hello,

The patch is explained below.


This patch looks sensible to me. Tim, any objection ?

Also, why use ${\cTAINT} instead of ${^TAINT} ? for older perls I suppose ?


I replied on the RT page. I've committed the first part to my local CVS 
and it will be in the next release. I'm thinking about the TAINT issue 
since not all operating systems support the MEDIUM and HIGH mode and I 
don't want to break them suddenly when they start using the module with 
taint mode enabled.


Tim


--
Tim Jenness
JAC software
http://www.jach.hawaii.edu/~timj

Re: File::Temp: euid in _is_safe(); safe_level(MEDIUM) for taint mode

2005-09-02 Thread Rafael Garcia-Suarez 
On 9/2/05, Tim Jenness [EMAIL PROTECTED] wrote:
 I replied on the RT page. I've committed the first part to my local CVS
 and it will be in the next release.

Oh, ok, the CPAN RT page I presume, since there is to ticket for that
on rt.perl.org.

So, everybody, just make my pumpking life a bit easier. I can't track
every single dual-life module on CPAN. My pool of information is the
P5P mailing list. Post your information here. /rant

Anyway, thanks, and I'll wait for your next release.

File::Temp: euid in _is_safe(); safe_level(MEDIUM) for taint mode

2005-08-16 Thread Alexey Tourbin 
Hello,

The patch is explained below.

--- File/Temp.pm-   2005-04-03 15:27:16 +
+++ File/Temp.pm2005-08-16 22:50:39 +
@@ -679,11 +679,11 @@ sub _is_safe {
   return 1 if $^O eq 'VMS';  # owner delete control at file level
 
   # Check to see whether owner is neither superuser (or a system uid) nor me
-  # Use the real uid from the $ variable
+  # Use the effective uid from the $ variable
   # UID is in [4]
-  if ($info[4]  File::Temp-top_system_uid()  $info[4] != $) {
+  if ($info[4]  File::Temp-top_system_uid()  $info[4] != $) {
 
-Carp::cluck(sprintf uid=$info[4] topuid=%s \$=$ path='$path',
+Carp::cluck(sprintf st_uid=$info[4] topuid=%s euid=$ path='$path',
File::Temp-top_system_uid());
 
 $$err_ref = Directory owned neither by root nor the current user
@@ -2241,4 +2241,10 @@ security enhancements.
 
 =cut
 
+{
+no strict 'refs';
+File::Temp-safe_level(MEDIUM)
+if ${\cTAINT};
+}
+
 1;
End of patch

First, real/effective UID distinction is essential for setuid scripts.
Filesystem permissions are controlled by the effective UID of the
process.  When a privileged script is checking if the directory is safe,
it should check that the directory is *not* owned by the caller.
Otherwise, the user can replace a temporary file created by the
privileged process, which is almost certainly not what we want.

Second, I suggest to enable MEDUM security level for taint mode,
which is on when running setuid/setgid scripts.  It's only on MEDUM
level that the above _is_safe() security check is performed.


pgpCpP9VmHh0V.pgp
Description: PGP signature