Roundup Tracker - Issues

Message5777

Author rouilj
Recipients antmail, joseph_myers, rouilj, schlatterbeck
Date 2016-07-07.23:52:50
Message-id <1467935571.69.0.846553576489.issue2550891@psf.upfronthosting.co.za>
In-reply-to
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?
History
Date User Action Args
2016-07-07 23:52:51rouiljsetmessageid: <1467935571.69.0.846553576489.issue2550891@psf.upfronthosting.co.za>
2016-07-07 23:52:51rouiljsetrecipients: + rouilj, schlatterbeck, joseph_myers, antmail
2016-07-07 23:52:51rouiljlinkissue2550891 messages
2016-07-07 23:52:50rouiljcreate