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

In reply to Re: security and un-tainting paths by graff
in thread security and un-tainting paths by wrinkles

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":


  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.