Do not fail on bad message pack message (bsc#1213441, CVE-2023-20897)

salt/channel/server.py

@@ -22,6 +22,7 @@
 import salt.utils.platform
 import salt.utils.stringutils
 import salt.utils.verify
+from salt.exceptions import SaltDeserializationError
 from salt.utils.cache import CacheCli
 
 try:
@@ -252,6 +253,15 @@ def _update_aes(self):
         return False
 
     def _decode_payload(self, payload):
+        # Sometimes msgpack deserialization of random bytes could be successful,
+        # so we need to ensure payload in good shape to process this function.
+        if (
+            not isinstance(payload, dict)
+            or "enc" not in payload
+            or "load" not in payload
+        ):
+            raise SaltDeserializationError("bad load received on socket!")
+
         # we need to decrypt it
         if payload["enc"] == "aes":
             try:

salt/transport/zeromq.py

@@ -428,7 +428,11 @@ def post_fork(self, message_handler, io_loop):
 
     @salt.ext.tornado.gen.coroutine
     def handle_message(self, stream, payload):
-        payload = self.decode_payload(payload)
+        try:
+            payload = self.decode_payload(payload)
+        except salt.exceptions.SaltDeserializationError:
+            self.stream.send(self.encode_payload({"msg": "bad load"}))
+            return
         # XXX: Is header really needed?
         reply = yield self.message_handler(payload)
         self.stream.send(self.encode_payload(reply))

tests/pytests/unit/transport/test_zeromq.py

@@ -11,6 +11,7 @@
 import time
 import uuid
 
+import msgpack
 import pytest
 
 import salt.channel.client
@@ -1404,3 +1405,71 @@ async def test_req_chan_auth_v2_new_minion_without_master_pub(pki_dir, io_loop):
     assert "sig" in ret
     ret = client.auth.handle_signin_response(signin_payload, ret)
     assert ret == "retry"
+
+
+async def test_req_server_garbage_request(io_loop):
+    """
+    Validate invalid msgpack messages will not raise exceptions in the
+    RequestServers's message handler.
+    """
+    opts = salt.config.master_config("")
+    request_server = salt.transport.zeromq.RequestServer(opts)
+
+    def message_handler(payload):
+        return payload
+
+    request_server.post_fork(message_handler, io_loop)
+
+    byts = msgpack.dumps({"foo": "bar"})
+    badbyts = byts[:3] + b"^M" + byts[3:]
+
+    valid_response = msgpack.dumps({"msg": "bad load"})
+
+    with MagicMock() as stream:
+        request_server.stream = stream
+
+        try:
+            await request_server.handle_message(stream, badbyts)
+        except Exception as exc:  # pylint: disable=broad-except
+            pytest.fail("Exception was raised {}".format(exc))
+
+    request_server.stream.send.assert_called_once_with(valid_response)
+
+
+async def test_req_chan_bad_payload_to_decode(pki_dir, io_loop):
+    opts = {
+        "master_uri": "tcp://127.0.0.1:4506",
+        "interface": "127.0.0.1",
+        "ret_port": 4506,
+        "ipv6": False,
+        "sock_dir": ".",
+        "pki_dir": str(pki_dir.joinpath("minion")),
+        "id": "minion",
+        "__role": "minion",
+        "keysize": 4096,
+        "max_minions": 0,
+        "auto_accept": False,
+        "open_mode": False,
+        "key_pass": None,
+        "publish_port": 4505,
+        "auth_mode": 1,
+        "acceptance_wait_time": 3,
+        "acceptance_wait_time_max": 3,
+    }
+    SMaster.secrets["aes"] = {
+        "secret": multiprocessing.Array(
+            ctypes.c_char,
+            salt.utils.stringutils.to_bytes(salt.crypt.Crypticle.generate_key_string()),
+        ),
+        "reload": salt.crypt.Crypticle.generate_key_string,
+    }
+    master_opts = dict(opts, pki_dir=str(pki_dir.joinpath("master")))
+    master_opts["master_sign_pubkey"] = False
+    server = salt.channel.server.ReqServerChannel.factory(master_opts)
+
+    with pytest.raises(salt.exceptions.SaltDeserializationError):
+        server._decode_payload(None)
+    with pytest.raises(salt.exceptions.SaltDeserializationError):
+        server._decode_payload({})
+    with pytest.raises(salt.exceptions.SaltDeserializationError):
+        server._decode_payload(12345)