CVE-2017-14696: Improper Input Validation (CWE-20)
value: SaltStack Salt before 2016.3.8, 2016.11.x before 2016.11.8, and 2017.7.x before 2017.7.2 allows remote attackers to cause a denial of service via a crafted authentication request.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 7.5
Published: 2017-10-24T17:29Z
Repository: https://github.com/saltstack/salt
Language: Python
Files changed: salt/crypt.py, salt/transport/tcp.py, salt/transport/zeromq.py
Commit date: 2017-10-03 18:17:19 -0400
Commit message: Do not allow IDs with null bytes in decoded payloads

Diff: diff --git a/salt/crypt.py b/salt/crypt.py
index f4f65940d59..ef4e7645d23 100644
--- a/salt/crypt.py
+++ b/salt/crypt.py
@@ -605,10 +605,13 @@ class AsyncAuth(object):
                 raise tornado.gen.Return('retry')
             if self.opts.get('detect_mode') is True:
                 raise tornado.gen.Return('retry')
             else:
                 raise SaltClientError('Attempt to authenticate with the salt master failed with timeout error')
+        if not isinstance(payload, dict):
+            log.error('Sign-in attempt failed: %s', payload)
+            raise tornado.gen.Return(False)
         if 'load' in payload:
             if 'ret' in payload['load']:
                 if not payload['load']['ret']:
                     if self.opts['rejected_retry']:
                         log.error(
diff --git a/salt/transport/tcp.py b/salt/transport/tcp.py
index a9001f03a5c..f274240a1eb 100644
--- a/salt/transport/tcp.py
+++ b/salt/transport/tcp.py
@@ -621,10 +621,21 @@ class TCPReqServerChannel(salt.transport.mixins.auth.AESReqServerMixin, salt.tra
             if not isinstance(payload, dict) or not isinstance(payload.get('load'), dict):
                 yield stream.write(salt.transport.frame.frame_msg(
                     'payload and load must be a dict', header=header))
                 raise tornado.gen.Return()
 
+            try:
+                id_ = payload['load'].get('id', '')
+                if '\0' in id_:
+                    log.error('Payload contains an id with a null byte: %s', payload)
+                    stream.send(self.serial.dumps('bad load: id contains a null byte'))
+                    raise tornado.gen.Return()
+            except TypeError:
+                log.error('Payload contains non-string id: %s', payload)
+                stream.send(self.serial.dumps('bad load: id {0} is not a string'.format(id_)))
+                raise tornado.gen.Return()
+
             # intercept the "_auth" commands, since the main daemon shouldn't know
             # anything about our key auth
             if payload['enc'] == 'clear' and payload.get('load', {}).get('cmd') == '_auth':
                 yield stream.write(salt.transport.frame.frame_msg(
                     self._auth(payload['load']), header=header))
diff --git a/salt/transport/zeromq.py b/salt/transport/zeromq.py
index 1bb3abebe1d..2be6c829f35 100644
--- a/salt/transport/zeromq.py
+++ b/salt/transport/zeromq.py
@@ -594,10 +594,21 @@ class ZeroMQReqServerChannel(salt.transport.mixins.auth.AESReqServerMixin, salt.
         if not isinstance(payload, dict) or not isinstance(payload.get('load'), dict):
             log.error('payload and load must be a dict. Payload was: {0} and load was {1}'.format(payload, payload.get('load')))
             stream.send(self.serial.dumps('payload and load must be a dict'))
             raise tornado.gen.Return()
 
+        try:
+            id_ = payload['load'].get('id', '')
+            if '\0' in id_:
+                log.error('Payload contains an id with a null byte: %s', payload)
+                stream.send(self.serial.dumps('bad load: id contains a null byte'))
+                raise tornado.gen.Return()
+        except TypeError:
+            log.error('Payload contains non-string id: %s', payload)
+            stream.send(self.serial.dumps('bad load: id {0} is not a string'.format(id_)))
+            raise tornado.gen.Return()
+
         # intercept the "_auth" commands, since the main daemon shouldn't know
         # anything about our key auth
         if payload['enc'] == 'clear' and payload.get('load', {}).get('cmd') == '_auth':
             stream.send(self.serial.dumps(self._auth(payload['load'])))
             raise tornado.gen.Return()

Fixed code:         if not isinstance(payload, dict):
            log.error('Sign-in attempt failed: %s', payload)
            raise tornado.gen.Return(False)
            try:
                id_ = payload['load'].get('id', '')
                if '\0' in id_:
                    log.error('Payload contains an id with a null byte: %s', payload)
                    stream.send(self.serial.dumps('bad load: id contains a null byte'))
                    raise tornado.gen.Return()
            except TypeError:
                log.error('Payload contains non-string id: %s', payload)
                stream.send(self.serial.dumps('bad load: id {0} is not a string'.format(id_)))
                raise tornado.gen.Return()

        try:
            id_ = payload['load'].get('id', '')
            if '\0' in id_:
                log.error('Payload contains an id with a null byte: %s', payload)
                stream.send(self.serial.dumps('bad load: id contains a null byte'))
                raise tornado.gen.Return()
        except TypeError:
            log.error('Payload contains non-string id: %s', payload)
            stream.send(self.serial.dumps('bad load: id {0} is not a string'.format(id_)))
            raise tornado.gen.Return()