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

Re^3: and untainting

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

in reply to Re^2: and untainting
in thread and untainting

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.

Replies are listed 'Best First'.
Re^4: and untainting
by syphilis (Chancellor) on Jul 28, 2009 at 23:45 UTC
    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.


      Thank you so much for responding - and for taking on the responsibility to maintain this module.

      IMHO the most important thing near term is to update the documentation so that it clearly describes the security issues along with recommendations for safe use. It needs to be explained more clearly that

      • SAFEMODE is not the least bit comprehensive. This can be nicely spun as "future implementation". All the same, I think one needs to make it very clear that some really obvious kinds of security are not being performed. For example, while reading through the code, I also noticed that rmpath calls _rmtree which disables taint protection on parameters to unlink by blindly passing them through /.*/. It also allows the entire tree under "/" to be deleted. There are probably others as well.
      • UNTAINT mode is optional - as CountZero points out the documentation implies the exact opposite. The only way to know that UNTAINT is optional is to scan the source code.
      • Using the UNTAINT option, effectively nukes Taint mode for the *entire* application, not just the portions controlled by Inline. This could have serious implications, especially in web applications that may be counting on Taint mode to break system calls using Environment variables.
      • I'd also add a note on the bug list about the security issues in the work-around for bug 13084 (Inline doesn't work in taint mode). The work-around suggested there nixes the only safety feature that SAFEMODE currently implements (it recommends setting CLEAN_AFTER_BUILD => 0).

      Also I would think twice about rushing to fix that UNTAINT bug. I see the long standing nature of that bug as good news. It means that security conscious developers are unlikely to be using this module in security sensitive production applications to any great degree simply because they can't. This has two benefits:

      • We have probably avoided some significant bad publicity about security issues in Perl's cross language support.
      • If you really do want to make this module work in taint mode, it buys you time until you or a volunteer is available to rethink the strategy for making this module work when TAINT mode is turned on. The current approach effectively boils down to "go away taint mode - I don't like you - you are in my way!". A developer (or corporation) that turns on taint mode is making a statement about their desired level of security. For those users, making a better sledge hammer to nuke taint mode is not going to be acceptable.

      Best, beth

        I've no problem with providing documentation that warns of the potential danger - in fact, mainly as a result of your suggestions, I'm now also thinking of having the use of both UNTAINT and SAFEMODE options generate a warning about the inadvisability of using them.

        I left things as they are (wrt tainting) in the last Inline::C update, partly to give me more time to think about what to do with it ... and I'm still finding most options unpalatable.

        I don't like the idea of just leaving it as is - that seems silly to me (despite the pragmatic wisdom). Surely it should be either fixed or removed.
        I certainly have no intention of personally trying to add improvements (that would be disastrous), and yet I consider that I would be acting in bad faith if I just went ahead and removed all of that code that Ingy (I presume it was he) had gone to so much trouble to put in place in the beginning.

        So, I still find myself leaning towards applying Patrick's patches - but, yes, with stern warnings about the dangers of using this particular piece of rope. (Thanks for the cautionary advice.)

        Ingy was recently making noises about once again contributing to Inline - I might yet try and contact him and find out just what his vision for untainting actually was, and how he thinks it should be dealt with in the present and future.


Log In?

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

How do I use this? | Other CB clients
Other Users?
Others having an uproarious good time at the Monastery: (7)
As of 2018-01-19 21:47 GMT
Find Nodes?
    Voting Booth?
    How did you see in the new year?

    Results (223 votes). Check out past polls.