salt/fixes-cve-2018-15750-cve-2018-15751.patch

From 43b1f8fb6608c944812bc5bcd9da407624409ac7 Mon Sep 17 00:00:00 2001
From: Erik Johnson <palehose@gmail.com>
Date: Fri, 24 Aug 2018 10:35:55 -0500
Subject: [PATCH] Fixes: CVE-2018-15750, CVE-2018-15751

Ensure that tokens are hex to avoid hanging/errors in cherrypy

Add empty token salt-api integration tests

Handle Auth exceptions in run_job

Update tornado test to correct authentication message
---
 salt/client/__init__.py                       |  8 ++++
 salt/netapi/rest_cherrypy/app.py              | 13 ++++++-
 .../netapi/rest_cherrypy/test_app.py          | 39 +++++++++++++++++++
 .../netapi/rest_tornado/test_app.py           |  2 +-
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/salt/client/__init__.py b/salt/client/__init__.py
index dcbc1473e1..77f2a963f7 100644
--- a/salt/client/__init__.py
+++ b/salt/client/__init__.py
@@ -349,6 +349,10 @@ class LocalClient(object):
             raise SaltClientError(
                 'The salt master could not be contacted. Is master running?'
             )
+        except AuthenticationError as err:
+            raise AuthenticationError(err)
+        except AuthorizationError as err:
+            raise AuthorizationError(err)
         except Exception as general_exception:
             # Convert to generic client error and pass along message
             raise SaltClientError(general_exception)
@@ -415,6 +419,10 @@ class LocalClient(object):
             raise SaltClientError(
                 'The salt master could not be contacted. Is master running?'
             )
+        except AuthenticationError as err:
+            raise AuthenticationError(err)
+        except AuthorizationError as err:
+            raise AuthorizationError(err)
         except Exception as general_exception:
             # Convert to generic client error and pass along message
             raise SaltClientError(general_exception)
diff --git a/salt/netapi/rest_cherrypy/app.py b/salt/netapi/rest_cherrypy/app.py
index 78ea3c3fef..c272674146 100644
--- a/salt/netapi/rest_cherrypy/app.py
+++ b/salt/netapi/rest_cherrypy/app.py
@@ -1167,6 +1167,13 @@ class LowDataAdapter(object):
             if token:
                 chunk['token'] = token
 
+            if 'token' in chunk:
+                # Make sure that auth token is hex
+                try:
+                    int(chunk['token'], 16)
+                except (TypeError, ValueError):
+                    raise cherrypy.HTTPError(401, 'Invalid token')
+
             if client:
                 chunk['client'] = client
 
@@ -2167,7 +2174,11 @@ class Events(object):
 
         :return bool: True if valid, False if not valid.
         '''
-        if auth_token is None:
+        # Make sure that auth token is hex. If it's None, or something other
+        # than hex, this will raise a ValueError.
+        try:
+            int(auth_token, 16)
+        except ValueError:
             return False
 
         # First check if the given token is in our session table; if so it's a
diff --git a/tests/integration/netapi/rest_cherrypy/test_app.py b/tests/integration/netapi/rest_cherrypy/test_app.py
index 000b7418bf..5865510fd7 100644
--- a/tests/integration/netapi/rest_cherrypy/test_app.py
+++ b/tests/integration/netapi/rest_cherrypy/test_app.py
@@ -124,6 +124,45 @@ class TestRun(cptc.BaseRestCherryPyTest):
         })
         self.assertEqual(response.status, '401 Unauthorized')
 
+    def test_run_empty_token(self):
+        '''
+        Test the run URL with empty token
+        '''
+        cmd = dict(self.low, **{'token': ''})
+        body = urlencode(cmd)
+
+        request, response = self.request('/run', method='POST', body=body,
+            headers={
+                'content-type': 'application/x-www-form-urlencoded'
+        })
+        assert response.status == '401 Unauthorized'
+
+    def test_run_empty_token_upercase(self):
+        '''
+        Test the run URL with empty token with upercase characters
+        '''
+        cmd = dict(self.low, **{'ToKen': ''})
+        body = urlencode(cmd)
+
+        request, response = self.request('/run', method='POST', body=body,
+            headers={
+                'content-type': 'application/x-www-form-urlencoded'
+        })
+        assert response.status == '401 Unauthorized'
+
+    def test_run_wrong_token(self):
+        '''
+        Test the run URL with incorrect token
+        '''
+        cmd = dict(self.low, **{'token': 'bad'})
+        body = urlencode(cmd)
+
+        request, response = self.request('/run', method='POST', body=body,
+            headers={
+                'content-type': 'application/x-www-form-urlencoded'
+        })
+        assert response.status == '401 Unauthorized'
+
 
 class TestWebhookDisableAuth(cptc.BaseRestCherryPyTest):
 
diff --git a/tests/integration/netapi/rest_tornado/test_app.py b/tests/integration/netapi/rest_tornado/test_app.py
index beb085db1e..01abd354a7 100644
--- a/tests/integration/netapi/rest_tornado/test_app.py
+++ b/tests/integration/netapi/rest_tornado/test_app.py
@@ -237,7 +237,7 @@ class TestSaltAPIHandler(_SaltnadoIntegrationTestCase):
         self.assertEqual(len(ret), 3)  # make sure we got 3 responses
         self.assertIn('jid', ret[0])  # the first 2 are regular returns
         self.assertIn('jid', ret[1])
-        self.assertIn('Authentication error occurred.', ret[2])  # bad auth
+        self.assertIn('Failed to authenticate', ret[2])  # bad auth
         self.assertEqual(ret[0]['minions'], sorted(['minion', 'sub_minion', 'localhost']))
         self.assertEqual(ret[1]['minions'], sorted(['minion', 'sub_minion', 'localhost']))
 
-- 
2.17.1