Re: security and un-tainting paths

Have I done this untaint thing properly?

Your regexes are imposing sensible limits on the range of characters allowed for input, but you aren't ruling out things like "../../../../.." as part of the final resulting path string. More on this below.

Is the regex appropriate for capturing directories and filenames?

Provided that your directories and file names are in fact limited to using the characters that your regexes allow, yes. You are explicitly not allowing names that include spaces or any punctuation other than period and hyphen, and (IMHO) that's reasonable, prudent, and basically good.

But you really should guard against "../../../../.." -- I'd limit the range of acceptable paths further, to exclude any name that begins with "." -- change your regexes to:

    m/^(\w[\w.-]*)$/;  # path component must begin with alphnumeric
[download]

Has my effort improved security?

Well, if the only difference is that the earlier version used the tainted variables in the File::Spec->catfile() call, I think you probably have provided some extra protection, by virtue of limiting the variety of path names that can be passed to do -- but you really should fix the "../../.." problem.

Even after fixing that, there's still a question whether you've improved security enough. The basic risk that remains is whether a hacker is able to plant files and use this app to execute those files. The OP code is only using read access to the directories and files, but if you don't enforce a suitable level of protection for write access, files could be planted by other means, and executed via your app.

An alternative method for protection is for the app to know which paths and template files are safe, have those in a hash or other data structure, and "do" a path/file name selected from that structure (because it matched the input exactly).

For that matter, if the purpose of the "do" is to run a template for presenting content, why not use a real templating module (e.g. Template::Toolkit? Calling a template module method is likely safer than executing perl code directly from a file.

One last point: assuming this is a web app, I would hope that the caller is wrapping its call to this sub inside an "eval", to trap the croaking and present some appropriate response to the (misguided) client, rather than just dying. (Personally, I'd have this function return a recognizable "fail" result, and let the caller decide whether to die or not.)

Comment on Re: security and un-tainting paths Select or Download Code

Replies are listed 'Best First'.
Re^2: security and un-tainting paths by wrinkles (Pilgrim) on Oct 23, 2011 at 06:47 UTC
Graff, thanks for the detailed reply, very much appreciated. I will fix the ../ possibility in my regex. In fact, the application checks a configuration hash for allowed files, as you suggested. I had suspected that was a security feature, now I know for sure. The base template and page templates are generally hashrefs which specify HTML::Template templates and other data, but may also run perl code. It does look like the main executable is called by eval. My desire to run in taint mode is motivated more by my desire to catch newbie mistakes on my part, than by a distrust in the base application (though I do try to cultivate a healthy distrust by default). Again, thanks for your help, learning opportunities like this help me build a mental framework upon which I can develop with reading on my own.	[reply]

Replies are listed 'Best First'.

Re^2: security and un-tainting paths
by wrinkles (Pilgrim) on Oct 23, 2011 at 06:47 UTC

Graff, thanks for the detailed reply, very much appreciated.

I will fix the ../ possibility in my regex. In fact, the application checks a configuration hash for allowed files, as you suggested. I had suspected that was a security feature, now I know for sure.

The base template and page templates are generally hashrefs which specify HTML::Template templates and other data, but may also run perl code.

It does look like the main executable is called by eval. My desire to run in taint mode is motivated more by my desire to catch newbie mistakes on my part, than by a distrust in the base application (though I do try to cultivate a healthy distrust by default).

Again, thanks for your help, learning opportunities like this help me build a mental framework upon which I can develop with reading on my own.

[reply]