I  think  we  can  just throw away a parameter with double dot inside.
This is not a thing relative to URL, URL is always a classname. This is
parameter   of   class   viewer.  There is no need to parse it by path
rules.

I thing something like

              if name.find("..") != 0:
                return # will raise invalid template

will be enough.

> John Rouillard added the comment:

> This may be too simple but if we change cgi/templating.py's

> TALLoaderBase::_find with:

> class TALLoaderBase(LoaderBase):
>     """ Common methods for the legacy TAL loaders."""

>     def __init__(self, dir):
>         self.dir = dir

>     def _find(self, name):
>         """ Find template, return full path and filename of the
>             template if it is found, None otherwise."""

> +       realsrc = os.path.realpath(self.dir)
>         for extension in ['', '.html', '.xml']:
>             f = name + extension
>             src = os.path.join(self.dir, f)
> +            realpath = os.path.realpath(src)
> +            if string.find(realpath, realsrc) != 0:
> +                return # will raise invalid template
>             if os.path.exists(src):
>                 return (src, f)

> (Don't forget to 'import string' at the top of the file if you apply
> this.) The lines I added start with +.

> This does the following:

> Find the real path for the template directory removing all .., /./. //
> etc. from the name and resolve any symbolic links in the path.
> Do the same for the template argument.

> Then require the real path of the tracker directory to be a prefix of
> the requested template starting at the root. Since nothing after the
> realsrc directory can traverse above the realsrc root (since realpath
> removed .. and links) I think that means the template has to be
> under the tracker's html directory.

> There are performance implications of this, but we may be able to turn
> self.dir into realpath(self.dir) in __init__ and remove some calls to
> realpath for it. Alternatively we can cache realsrc in a class
> variable indexed by self.dir since I think we trust self.dir is not
> malicious.

> I think this will handle issue2550701 for the TAL based templates.
> Looking at engine_chameleon.py it looks like it uses TALLoaderBase's
> _find as well so....

> Some rudimentary testing after creating a directory: html/issue.item
> and a template value that via .. gets me to /etc/passwd was stopped by
> this with the following traceback:

>   File "roundup2.hg/roundup/cgi/client.py", line 1176, in selectTemplate
>     tplname, generic))
> NoTemplate: No template file exists for templating "issue" with
>     template "subdir/../../../../../../../../etc/passwd"
>     (neither "issue.subdir/../../../../../../../../etc/passwd" nor
>     "_generic.subdir/../../../../../../../../etc/passwd")

> Note I was in the directory roundup.hg (which is a symbolic link to
> roundup.hg2)

> Joseph Myers I am considering this to patch and close out
> issue2550701.

> Thoughts?

> ________________________________________________
> Roundup tracker <issues@roundup-tracker.org>
> <http://issues.roundup-tracker.org/issue2550891>
> ________________________________________________