Beefy Boxes and Bandwidth Generously Provided by pair Networks
No such thing as a small change
 
PerlMonks  

Inline.pm and untainting

by syphilis (Canon)
on Jul 28, 2009 at 11:55 UTC ( #783905=perlquestion: print w/ replies, xml ) Need Help??
syphilis has asked for the wisdom of the Perl Monks concerning the following question:

Hi,
In Inline.pm we have the following subroutine which gets called whenever the Config option 'UNTAINT' is set:
#===================================================================== +========= # Blindly untaint tainted fields in Inline object. #===================================================================== +========= sub env_untaint { my $o = shift; for (keys %ENV) { ($ENV{$_}) = $ENV{$_} =~ /(.*)/; } my $delim = $^O eq 'MSWin32' ? ';' : ':'; $ENV{PATH} = join $delim, grep {not /^\./ and -d $_ and not ((stat($_))[2] & 0022) } split $delim, $ENV{PATH}; map {($_) = /(.*)/} @INC; }
And that works quite well on linux, untainting $ENV{PATH} and leaving it intact. But on windows it clobbers $ENV{PATH}, leaving it empty.

The culprit is the not ((stat($_))[2] & 0022) condition. On linux, where stat($_))[2] is 16877 for all of the folders in my path, ((stat($_))[2] & 0022) is false. But on windows stat($_))[2] is 16895 for all of the folders in my path, and ((stat($_))[2] & 0022) is true.

If I simply remove that condition for windows, everything is fine. But is that the correct thing to do ? Why has that condition been put in there, and what's the correct windows form it should take ?

Cheers,
Rob

Comment on Inline.pm and untainting
Select or Download Code
Re: Inline.pm and untainting
by ELISHEVA (Prior) on Jul 28, 2009 at 12:17 UTC

    I can't comment to your question about stat, but one thing I notice that worries me is the way this subroutine is untainting. According to numerous sources, filters should specify a list of legal characters and/or patterns rather than rely on a list of bad characters. From our own perlsec:

    It's better to verify that the variable has only good characters (for certain values of "good") rather than checking whether it has any bad characters. That's because it's far too easy to miss bad characters that you never thought of.

    Merely excluding characters or looking for "." at the start of a path name can make you vulnerable to a host of security exploits. For example, your current filter would allow something like "/sbin", "/sbin\0bad stuff here" (poison null vulnerability) and, of course, "foo/././sbin" to be added to $PATH.

    Given that this is a published CPAN module, it might be a good idea to rethink that filtering strategy and maybe even submit a bug fix. Consider using another routine or doing your own filtering of $ENV{PATH}.

    Best, beth

      The docs of Inline says as follows:
      UNTAINT

      You must use this option whenever you use Perl's -T switch, for taint checking. This option tells Inline to blindly untaint all tainted variables. It also turns on SAFEMODE by default. (...)

      SAFEMODE

      Perform extra safety checking, in an attempt to thwart malicious code. This option cannot guarantee security, but it does turn on all the currently implemented checks.

      There is a slight startup penalty by using SAFEMODE. Also, using UNTAINT automatically turns this option on. If you need your code to start faster under -T (taint) checking, you'll need to turn this option off manually. Only do this if you are not worried about security risks.

      So you probably should not look at this code in isolation but together with the whole set-up of Inline. I agree it is strange that in order to be able to run Inline in taint mode you have to globally untaint all your environment variables.

      CountZero

      A program should be light and agile, its subroutines connected like a string of pearls. The spirit and intent of the program should be retained throughout. There should be neither too little or too much, neither needless loops nor useless variables, neither lack of structure nor overwhelming rigidity." - The Tao of Programming, 4.1 - Geoffrey James

        I may be misreading the source code, but it appears that the requirement to use UNTAINT when taint mode is turned on is in fact optional. Inline.pm does not check ^T (or ${^TAINT}) to see if taint mode is turned on, nor does it fail if taint mode is on but the module consumer fails to set the UNTAINT option. Thus the consumer module, may if it wishes provide an alternate untainting strategy. However, this strategy must be implemented in a BEGIN{...} block placed before use Inline.

        My guess is that the sledge hammer approach arises from the classic tension between ease of use and security. The import routine of Inline calls system (directly and via ExtUtils::MakeMaker) to prepare configuration files and compile any source code needed by the inlined subroutines. In taint mode there is a list of environment variables that will prevent system from working. In addition, many Perl language built-ins that make system calls will fail if passed tainted parameters. Given that many build process parameters are stored in environment variables, it may just have seemed easier to untaint the entire %ENV hash.

        As for SAFEMODE - that appears to be an idea that never fully got implemented. The only thing it appears to do is check to see if the DIRECTORY option is set. That option cleans out the build areas used by Inline so that no malicious garbage can be injected.

        I think it would be worth another set of monkish eyes to verify my observations about SAFEMODE and UNTAINT. This is one case where I would love to be wrong and simply have overlooked something.

        Best, beth

        Update: added comments about SAFEMODE and request for others to look at the source code.

Re: Inline.pm and untainting
by blahblahblah (Priest) on Jul 29, 2009 at 03:14 UTC
    I'm surprised nobody has answered you yet. I'll take a stab even though I'm far from an expert on file modes, umasks, and the like. I think that 16895 and 16877 are decimals which represent the permissions drwxrwxrwx and drwxr-xr-x. Here's the link from which I grabbed those values: http://www.madmongers.org/mailing-list/mailing-list/help-with-octal-math

    So I guess the & 0222 is making sure that each dir is not writable?

      I'm surprised nobody has answered you yet.

      On reflection, the title probably doesn't match the actual question (which boils down to directory permissions) all that well.

      So I guess the & 0222 is making sure that each dir is not writable?

      Thanks for that - that makes sense (typo - it's 0022, not 0222).

      So .... according to stat(), all directories on my windows Vista box (even those that I can't write to) are drwxrwxrwx.
      Does this signify some shortcoming of the stat function on Windows ?
      Or does it signify some problem with the way this box is set up ?
      Or does it mean something else altogether ?

      And what are the ramifications re the particular condition in sub env_untaint that's posing the problem ?
      I haven't found anything helpful about this in the stat or perlport documentation.

      Cheers,
      Rob
        So .... according to stat(), all directories on my windows Vista box (even those that I can't write to) are drwxrwxrwx. Does this signify some shortcoming of the stat function on Windows ?

        Yes. Windows uses ACLs, not the Unix idea of owning user, group members, and other people (u,g,o). stat() emulates just enough of the Unix idea to allow common programs to work. It pretends that everything is readable and writeable for everyone. Finding out that things may be read-only or even write-only is left to other functions like open, read, and print. If you would run on a Unix system with ACLs, you would have exactly the same situation. The mode information returned by stat does not know about ACLs, and even if stat() tells you a file or directory is readable or writable for everyone, the ACLs may prevent you from reading or writing.

        Actually, stat() returns some useful information:

        perl -e "printf qq[%08o %s\n],(stat($_))[2],$_ for @ARGV" C:\WINNT C:\ +WINNT\win.ini C:\WINNT\NOTEPAD.EXE c:\WINNT\system32\command.com c:\ +winnt\system32\login.cmd C:\bin\uuencode.pl 00040777 C:\WINNT 00100666 C:\WINNT\win.ini 00100777 C:\WINNT\NOTEPAD.EXE 00100777 c:\WINNT\system32\command.com 00100777 c:\winnt\system32\login.cmd 00100666 C:\bin\uuencode.pl

        (Strawberry Perl 5.10.0)

        As you see, the directory flag is correct, and the X-bit is only set for directories and executables (determinated by the file extension). So, -f, -d, and -x work as expected.

        Alexander

        --
        Today I will gladly share my knowledge and experience, for there are no sweeter words than "I told you so". ;-)

        I'm surprised that env_untaint was written that way. 0022 as a umask is 755 permission wise which gives the idea that r and x for group members and everyone else actually works for Windows in a Linux/UNIX way.

        Reading stat under perlfunc years ago led me to believe that only a handful of the 13 elements actually worked under Windows. Looking at it now hints that there maybe some cause for doubt - "Not all fields are supported on all filesystem types".

        The concept of group and everyone for Windows does work under domains but not directories, which is why stat returns 0777 even on readonly.

        Shortcoming of stat - I'd say so, but it's not actually since we're given warning. To get directories that you can't write to I'd use Win32::File so

        and not ((stat($_))[2] & 0022)

        becomes

        and ( $attr & READONLY )

        I was going to go through the bitwise & permissions that would yield 0 with 0022, but I actually came on PM to look for something else and now it's home time :p

        Just in

        P.S. your box is fine

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://783905]
Approved by marto
Front-paged by Corion
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others exploiting the Monastery: (5)
As of 2014-08-21 16:56 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The best computer themed movie is:











    Results (138 votes), past polls