File do-not-fail-on-bad-message-pack-message-bsc-1213441-.patch of Package venv-salt-minion
From da544d7ab09899717e57a02321928ceaf3c6465c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?=
<psuarezhernandez@suse.com>
Date: Tue, 22 Aug 2023 11:43:46 +0100
Subject: [PATCH] Do not fail on bad message pack message (bsc#1213441,
CVE-2023-20897) (#595)
* Do not fail on bad message pack message
Fix unit test after backporting to openSUSE/release/3006.0
* Better error message when inconsistent decoded payload
---------
Co-authored-by: Daniel A. Wozniak <dwozniak@vmware.com>
---
salt/channel/server.py | 10 +++
salt/transport/zeromq.py | 6 +-
tests/pytests/unit/transport/test_zeromq.py | 69 +++++++++++++++++++++
3 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/salt/channel/server.py b/salt/channel/server.py
index a2117f2934..b6d51fef08 100644
--- a/salt/channel/server.py
+++ b/salt/channel/server.py
@@ -22,6 +22,7 @@ import salt.utils.minions
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 @@ class ReqServerChannel:
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:
diff --git a/salt/transport/zeromq.py b/salt/transport/zeromq.py
index 3ec7f7726c..7cc6b9987f 100644
--- a/salt/transport/zeromq.py
+++ b/salt/transport/zeromq.py
@@ -428,7 +428,11 @@ class RequestServer(salt.transport.base.DaemonizedRequestServer):
@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))
diff --git a/tests/pytests/unit/transport/test_zeromq.py b/tests/pytests/unit/transport/test_zeromq.py
index 10bb4917b8..c7cbc53864 100644
--- a/tests/pytests/unit/transport/test_zeromq.py
+++ b/tests/pytests/unit/transport/test_zeromq.py
@@ -11,6 +11,7 @@ import threading
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)
--
2.41.0
