From 1dca2fc0ec4be2fd3d2a811148400ff033dc80ca Mon Sep 17 00:00:00 2001 From: rocknes Date: Fri, 20 Jul 2018 14:22:24 -0700 Subject: [PATCH 1/2] Security fix for tarfile.extractall(). --- pyvcloud/vcd/org.py | 4 ++- pyvcloud/vcd/utils.py | 58 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pyvcloud/vcd/org.py b/pyvcloud/vcd/org.py index 7ab1ccc50..7eb9fc998 100644 --- a/pyvcloud/vcd/org.py +++ b/pyvcloud/vcd/org.py @@ -40,6 +40,7 @@ from pyvcloud.vcd.exceptions import UploadException from pyvcloud.vcd.system import System from pyvcloud.vcd.utils import get_admin_href +from pyvcloud.vcd.utils import get_safe_members_in_tar_file from pyvcloud.vcd.utils import to_dict DEFAULT_CHUNK_SIZE = 1024 * 1024 @@ -575,7 +576,8 @@ def upload_ovf(self, try: tempdir = tempfile.mkdtemp(dir='.') ova = tarfile.open(file_name) - ova.extractall(path=tempdir) + ova.extractall(path=tempdir, + members=get_safe_members_in_tar_file(ova)) ova.close() ovf_file = None extracted_files = os.listdir(tempdir) diff --git a/pyvcloud/vcd/utils.py b/pyvcloud/vcd/utils.py index 3a10fedd6..5ecbbbd3d 100644 --- a/pyvcloud/vcd/utils.py +++ b/pyvcloud/vcd/utils.py @@ -13,6 +13,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +from os.path import abspath +from os.path import dirname +from os.path import join as joinpath +from os.path import realpath + import humanfriendly from lxml import etree from lxml.objectify import NoneElement @@ -641,3 +646,56 @@ def get_admin_extension_href(href): return href.replace('/api/admin/', '/api/admin/extension/') else: return href.replace('/api/', '/api/admin/extension/') + + +def _badpath(path, base): + """Determines if a given file path is under a given base path or not. + + :param str path: file path where the file will be extracted to. + :param str base: path to the current working directory. + + :return: False, if the path is under the given base, else True. + + :rtype: bool + """ + # joinpath will ignore base if path is absolute + return not realpath(abspath(joinpath(base, path))).startswith(base) + + +def _badlink(info, base): + """Determine if a given link is under a given base path or not. + + :param TarInfo info: file that is going to be extracted. + :param str base: path to the current working directory. + + :return: False, if the path is under the given base, else True. + + :rtype: bool + """ + # Links are interpreted relative to the directory containing the link + tip = realpath(abspath(joinpath(base, dirname(info.name)))) + return _badpath(info.linkname, base=tip) + + +def get_safe_members_in_tar_file(tarfile): + """Retrieve members of a tar file that are safe to extract. + + :param Tarfile tarfile: the archive that has been opened as a TarFile + object. + + :return: list of members in the archive that are safe to extract. + + :rtype: list + """ + base = realpath(abspath(('.'))) + result = [] + for finfo in tarfile.getmembers(): + if _badpath(finfo.name, base): + print(finfo.name + ' is blocked: illegal path.') + elif finfo.issym() and _badlink(finfo, base): + print(finfo.name + ' is blocked: Hard link to ' + finfo.linkname) + elif finfo.islnk() and _badlink(finfo, base): + print(finfo.name + ' is blocked: Symlink to ' + finfo.linkname) + else: + result.append(finfo) + return result From 3cc26c5ce7ddf7fc4cd57cb7e99f880cebc6b414 Mon Sep 17 00:00:00 2001 From: rocknes Date: Mon, 23 Jul 2018 16:29:42 -0700 Subject: [PATCH 2/2] Addressing review comments. --- pyvcloud/vcd/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyvcloud/vcd/utils.py b/pyvcloud/vcd/utils.py index 5ecbbbd3d..dad6d220a 100644 --- a/pyvcloud/vcd/utils.py +++ b/pyvcloud/vcd/utils.py @@ -693,9 +693,9 @@ def get_safe_members_in_tar_file(tarfile): if _badpath(finfo.name, base): print(finfo.name + ' is blocked: illegal path.') elif finfo.issym() and _badlink(finfo, base): - print(finfo.name + ' is blocked: Hard link to ' + finfo.linkname) - elif finfo.islnk() and _badlink(finfo, base): print(finfo.name + ' is blocked: Symlink to ' + finfo.linkname) + elif finfo.islnk() and _badlink(finfo, base): + print(finfo.name + ' is blocked: Hard link to ' + finfo.linkname) else: result.append(finfo) return result