CVE-2017-14695: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Directory traversal vulnerability in minion id validation in SaltStack Salt before 2016.3.8, 2016.11.x before 2016.11.8, and 2017.7.x before 2017.7.2 allows remote minions with incorrect credentials to authenticate to a master via a crafted minion ID.  NOTE: this vulnerability exists because of an incomplete fix for CVE-2017-12791.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 9.8
Published: 2017-10-24T17:29Z
Repository: https://github.com/saltstack/salt
Language: Python
Files changed: salt/utils/verify.py, tests/unit/utils/test_verify.py
Commit date: 2017-10-03 18:19:25 -0400
Commit message: Don't allow path separators in minion ID

Diff: diff --git a/salt/utils/verify.py b/salt/utils/verify.py
index 6e69283f073..24b2a1d962e 100644
--- a/salt/utils/verify.py
+++ b/salt/utils/verify.py
@@ -478,26 +478,19 @@ def clean_path(root, path, subdir=False):
         if os.path.dirname(path) == os.path.normpath(root):
             return path
     return ''
 
 
-def clean_id(id_):
-    '''
-    Returns if the passed id is clean.
-    '''
-    if re.search(r'\.\.\{sep}'.format(sep=os.sep), id_):
-        return False
-    return True
-
-
 def valid_id(opts, id_):
     '''
     Returns if the passed id is valid
     '''
     try:
-        return bool(clean_path(opts['pki_dir'], id_)) and clean_id(id_)
-    except (AttributeError, KeyError, TypeError) as e:
+        if any(x in id_ for x in ('/', '\\', '\0')):
+            return False
+        return bool(clean_path(opts['pki_dir'], id_))
+    except (AttributeError, KeyError, TypeError):
         return False
 
 
 def safe_py_code(code):
     '''
diff --git a/tests/unit/utils/test_verify.py b/tests/unit/utils/test_verify.py
index 795298877dd..fa091f6cb39 100644
--- a/tests/unit/utils/test_verify.py
+++ b/tests/unit/utils/test_verify.py
@@ -56,10 +56,20 @@ class TestVerify(TestCase):
         Refs #8259
         '''
         opts = {'pki_dir': '/tmp/whatever'}
         self.assertFalse(valid_id(opts, None))
 
+    def test_valid_id_pathsep(self):
+        '''
+        Path separators in id should make it invalid
+        '''
+        opts = {'pki_dir': '/tmp/whatever'}
+        # We have to test both path separators because os.path.normpath will
+        # convert forward slashes to backslashes on Windows.
+        for pathsep in ('/', '\\'):
+            self.assertFalse(valid_id(opts, pathsep.join(('..', 'foobar'))))
+
     def test_zmq_verify(self):
         self.assertTrue(zmq_version())
 
     def test_zmq_verify_insufficient(self):
         import zmq

Vulnerable code: def clean_id(id_):
    '''
    Returns if the passed id is clean.
    '''
    if re.search(r'\.\.\{sep}'.format(sep=os.sep), id_):
        return False
    return True


        return bool(clean_path(opts['pki_dir'], id_)) and clean_id(id_)
    except (AttributeError, KeyError, TypeError) as e:
Fixed code:         if any(x in id_ for x in ('/', '\\', '\0')):
            return False
        return bool(clean_path(opts['pki_dir'], id_))
    except (AttributeError, KeyError, TypeError):
    def test_valid_id_pathsep(self):
        '''
        Path separators in id should make it invalid
        '''
        opts = {'pki_dir': '/tmp/whatever'}
        # We have to test both path separators because os.path.normpath will
        # convert forward slashes to backslashes on Windows.
        for pathsep in ('/', '\\'):
            self.assertFalse(valid_id(opts, pathsep.join(('..', 'foobar'))))