Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

[VCDA-707] Fixed security hole in tarfile.extractall() related to directory walking. #268

Merged
merged 2 commits into from
Jul 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pyvcloud/vcd/org.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
58 changes: 58 additions & 0 deletions pyvcloud/vcd/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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: 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