Beefy Boxes and Bandwidth Generously Provided by pair Networks
Just another Perl shrine

Re: and untainting

by ELISHEVA (Prior)
on Jul 28, 2009 at 12:17 UTC ( #783907=note: print w/replies, xml ) Need Help??

in reply to and untainting

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

Replies are listed 'Best First'.
Re^2: and untainting
by CountZero (Bishop) on Jul 28, 2009 at 13:18 UTC
    The docs of Inline says as follows:

    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. (...)


    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.


    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. 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.

        As for SAFEMODE .... The only thing it appears to do is check to see if the DIRECTORY option is set

        Yes, that's all it does.

        There's a bug report about the failure of Inline to work as intended wrt the UNTAINT option, which was submitted in June 2005. I think it's about time to fix that bug, to the extent that the behaviour is as the original author intended (no more, no less). I think it's ridiculous to leave a feature in an unusable state for such a length of time, irrespective of the value of that feature. (I'm the current maintainer, btw.)
        Patrick LeBoutillier has kindly written patches that fix the problem - except that the windows-specific aspect I've asked about here is not dealt with.

        I don't understand taint mode all that well - I certainly don't know what that not ((stat($_))[2] & 0022) stuff is all about, and I don't need to know. Assuming it does something valid, I just need to know its windows equivalent :-) Anyone ?

        At some stage in the future, someone who cares might provide enhancements to Inline's handling of taint mode - and such patches would be received gladly. But for the moment I'd just like to see it working as currently intended (and that ancient bug report closed). As noted, one is not forced to use the UNTAINT option.


Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://783907]
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others wandering the Monastery: (8)
As of 2018-03-17 15:34 GMT
Find Nodes?
    Voting Booth?
    When I think of a mole I think of:

    Results (224 votes). Check out past polls.