Check tarballs before extracting

This resolves #226

This change does the following:
1. Replace python's tarfile with a call to the system's tar
   utility. We do this to take advantage of the CVE-2013-4420 fix
   to libtar. Python's tarfile module has a fix in the workings
   but is yet to be merged as of this change.
2. We check for EOF errors and empty tarballs. This is mostly to
   address a few instances where we have seen Docker images that
   were malformed.
3. We modify some functions around loading and analyzing Docker
   images and layers including catching the extra errors that we
   raise for No. 2.

- rootfs: Moved some functionality from check_tar_permissions into
  a new function called shell_command. This function simply runs
  shell commands as the current user and returns the result and
  error to be dealt by the calling function.
- rootfs: check_tar_permissions will now use shell_command.
- rootfs: Created a new function called check_tar_members which
  will list the elements in the tarball to see if there are any
  EOF or empty tarballs.
- rootfs: Repurposed extract_layer_tar to be a general purpose
  extract_tarfile function which can be used throughout the code.
- container: Use extract_tarfile to extract image metadata.
- image_layer: Use extract_tarfile to extract image layer tarballs.
- analyze: Set up mount points after the image is loaded.
- report: In general setup, don't create directories. extract_tarfile
  will now do it.
- report: Catch all the appropriate errors that might get thrown when
  trying to load an image.

Signed-off-by: Nisha K <[email protected]>

tern/classes/image_layer.py

@@ -3,7 +3,6 @@
 # Copyright (c) 2017-2019 VMware, Inc. All Rights Reserved.
 # SPDX-License-Identifier: BSD-2-Clause
 
-import os
 
 from tern.classes.package import Package
 from tern.classes.origins import Origins
@@ -170,8 +169,5 @@ def gen_fs_hash(self):
         if self.__tar_file:
             fs_dir = rootfs.get_untar_dir(self.__tar_file)
             tar_file = rootfs.get_layer_tar_path(self.__tar_file)
-            # remove the fs directory if it already exists
-            if os.path.isdir(fs_dir):
-                rootfs.root_command(rootfs.remove, fs_dir)
-            rootfs.extract_layer_tar(tar_file, fs_dir)
+            rootfs.extract_tarfile(tar_file, fs_dir)
             self.__fs_hash = rootfs.calc_fs_hash(fs_dir)

tern/report/analyze.py

@@ -59,6 +59,8 @@ def prepare_for_analysis(image_obj, dockerfile):
         dhelper.set_imported_layers(image_obj)
     # add notices for each layer if it is imported
     image_setup(image_obj)
+    # set up the mount points
+    rootfs.set_up()
 
 
 def analyze_first_layer(image_obj, master_list, redo):

tern/report/report.py

@@ -65,11 +65,6 @@ def setup(dockerfile=None, image_tag_string=None):
                 logger.fatal("%s", errors.cannot_find_image.format(
                     imagetag=image_tag_string))
                 sys.exit()
-    # create temporary working directory
-    if not os.path.exists(constants.temp_folder):
-        os.mkdir(constants.temp_folder)
-    # set up folders for rootfs operations
-    rootfs.set_up()
 
 
 def teardown():
@@ -117,19 +112,14 @@ def load_base_image():
                 imagetag=base_image.repotag))
     try:
         base_image.load_image()
-    except NameError as error:
+    except (NameError,
+            subprocess.CalledProcessError,
+            IOError,
+            ValueError,
+            EOFError) as error:
         logger.warning('Error in loading base image: %s', str(error))
         base_image.origins.add_notice_to_origins(
             dockerfile_lines, Notice(str(error), 'error'))
-    except subprocess.CalledProcessError as error:
-        logger.warning(
-            'Error in loading base image: %s', str(error.output, 'utf-8'))
-        base_image.origins.add_notice_to_origins(
-            dockerfile_lines, Notice(str(error.output, 'utf-8'), 'error'))
-    except IOError as error:
-        logger.warning('Error in loading base image: %s', str(error))
-        base_image.origins.add_notice_to_origin(
-            dockerfile_lines, Notice(str(error), 'error'))
     return base_image
 
 
@@ -140,13 +130,12 @@ def load_full_image(image_tag_string):
         testimage=test_image.repotag)
     try:
         test_image.load_image()
-    except NameError as error:
-        test_image.origins.add_notice_to_origins(
-            failure_origin, Notice(str(error), 'error'))
-    except subprocess.CalledProcessError as error:
-        test_image.origins.add_notice_to_origins(
-            failure_origin, Notice(str(error.output, 'utf-8'), 'error'))
-    except IOError as error:
+    except (NameError,
+            subprocess.CalledProcessError,
+            IOError,
+            ValueError,
+            EOFError) as error:
+        logger.warning('Error in loading image: %s', str(error))
         test_image.origins.add_notice_to_origins(
             failure_origin, Notice(str(error), 'error'))
     return test_image

tern/utils/container.py

@@ -12,7 +12,6 @@
 import logging
 import os
 import pwd
-import tarfile
 import time
 from requests.exceptions import HTTPError
 
@@ -21,6 +20,7 @@
 from tern.utils.constants import logger_name
 from tern.utils.constants import temp_folder
 from tern.utils.constants import temp_tarfile
+from tern.utils import rootfs
 
 
 # timestamp tag
@@ -64,8 +64,8 @@ def check_container():
 
 def check_image(image_tag_string):
     '''Check if image exists'''
-    logger.debug("Checking if image \"%s\" is available on disk...",
-        image_tag_string)
+    logger.debug(
+        "Checking if image \"%s\" is available on disk...", image_tag_string)
     try:
         client.images.get(image_tag_string)
         logger.debug("Image \"%s\" found", image_tag_string)
@@ -145,11 +145,10 @@ def extract_image_metadata(image_tag_string):
             for chunk in result:
                 f.write(chunk)
         # extract tarfile into folder
-        with tarfile.open(temp_tarfile) as tar:
-            tar.extractall(temp_path)
+        rootfs.extract_tarfile(temp_tarfile, temp_path)
         # remove temporary tar file
         os.remove(temp_tarfile)
-        if not os.path.exists(temp_path):
+        if not os.listdir(temp_path):
             raise IOError('Unable to untar Docker image')
     except docker.errors.APIError:  # pylint: disable=try-except-raise
         raise

tern/utils/rootfs.py

@@ -19,6 +19,7 @@
 remove = ['rm', '-rf']
 
 # tar commands
+check_tar = ['tar', '-tf']
 extract_tar = ['tar', '-xf']
 
 # mount commands
@@ -27,7 +28,6 @@
 mount_sys = ['mount', '-o', 'bind', '/sys']
 mount_dev = ['mount', '-o', 'bind', '/dev']
 unmount = ['umount']
-extract_tar = ['tar', '-xvf']
 
 # enable host DNS settings
 host_dns = ['cp', constants.resolv_path]
@@ -66,9 +66,9 @@ def root_command(command, *extra):
     return result
 
 
-def check_command_permissions(command, *extra):
-    '''Invoke a shell command as the current user. If the error contains
-    'Operation not permitted' then return False. Else return True'''
+def shell_command(command, *extra):
+    '''Invoke a shell command as a regular user.
+    This is used to check the result and error message of the command'''
     full_cmd = []
     full_cmd.extend(command)  # we do this because command may be used again
     for arg in extra:
@@ -77,12 +77,30 @@ def check_command_permissions(command, *extra):
     logger.debug("Running command: %s", ' '.join(full_cmd))
     pipes = subprocess.Popen(full_cmd, stdout=subprocess.PIPE,  # nosec
                              stderr=subprocess.PIPE)
-    _, error = pipes.communicate()  # nosec
+    return pipes.communicate()  # nosec
+
+
+def check_tar_permissions(tar_file, directory_path):
+    '''Invoke a shell command as the current user. If the error contains
+    'Operation not permitted' then return False. Else return True'''
+    _, error = shell_command(extract_tar, tar_file, '-C', directory_path)
     if "Operation not permitted" in error.decode():
         return False
     return True
 
 
+def check_tar_members(tar_file):
+    '''Given the path to the tar file, check to see if there is an error with
+    the members of the tarfile or if it is empty'''
+    result, error = shell_command(check_tar, tar_file)
+    if error:
+        logger.error("Malformed tar: %s", error.decode())
+        raise EOFError("Malformed tarball: {}".format(tar_file))
+    if not result:
+        raise ValueError("Empty tarball: {}".format(tar_file))
+    return result
+
+
 def get_untar_dir(layer_tarfile):
     '''get the directory to untar the layer tar file'''
     return os.path.join(constants.temp_folder, os.path.dirname(
@@ -104,9 +122,9 @@ def set_up():
         os.mkdir(mergedir_path)
 
 
-def extract_layer_tar(layer_tar_path, directory_path):
-    '''Assuming all the metadata for an image has been extracted into the
-    temp folder, extract the tarfile into the required directory'''
+def extract_tarfile(tar_path, directory_path):
+    '''Give the full path to the tar file, extract the tar file into the
+    given directory'''
     try:
         os.mkdir(directory_path)
     except FileExistsError:
@@ -118,12 +136,13 @@ def extract_layer_tar(layer_tar_path, directory_path):
             # attempt to remove using root permissions
             root_command(remove, directory_path)
             os.mkdir(directory_path)
+    # check tarball - should raise an error if anything is wrong
+    check_tar_members(tar_path)
     # check if user can extract tarball
-    success = check_command_permissions(
-        extract_tar, layer_tar_path, '-C', directory_path)
+    success = check_tar_permissions(tar_path, directory_path)
     if not success:
         # attempt to extract using root permissions
-        root_command(extract_tar, layer_tar_path, '-C', directory_path)
+        root_command(extract_tar, tar_path, '-C', directory_path)
 
 
 def prep_rootfs(rootfs_dir):