Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAS-132669 / 25.04 / Make parse_message ensure that the message is a dict #15377

Merged
merged 4 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -283,13 +283,9 @@ async def validate_message(message: Any) -> None:
except KeyError:
message["params"] = []

async def process_message(self, app: RpcWebSocketApp, message: Any):
async def process_message(self, app: RpcWebSocketApp, message: dict):
try:
await self.validate_message(message)
except TypeError:
# TypeError here means message doesn't adhere to minimum
# format of the message (i.e. needs to be a dict)
app.send_error(None, JSONRPCError.INVALID_REQUEST.value, "Invalid Message Format")
except ValueError as e:
if (id_ := message.get("id", undefined)) != undefined:
app.send_error(id_, JSONRPCError.INVALID_REQUEST.value, str(e))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,7 @@
ValueError,
),
# fuzzy
("", TypeError),
({}, ValueError),
(None, TypeError),
([], TypeError),
(["bad"], TypeError),
(b"bad", TypeError),
],
)
@pytest.mark.asyncio
Expand Down
18 changes: 17 additions & 1 deletion src/middlewared/middlewared/pytest/unit/utils/test_limits.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import pytest
import json

from aiohttp.http_websocket import WSCloseCode
import pytest

from middlewared.utils import limits


Expand Down Expand Up @@ -49,3 +50,18 @@ def test__limit_authenticated_parse():
data = {'msg': 'method', 'method': 'canary', 'params': ['x' * 1000]}
parsed = limits.parse_message(True, json.dumps(data))
assert parsed == data


@pytest.mark.parametrize("value", [False, 0, "0"])
def test__invalid_type(value):
with pytest.raises(ValueError) as ve:
limits.parse_message(True, json.dumps(value))

assert ve.value.args[0] == "Invalid Message Format"


def test__invalid_type__list():
with pytest.raises(ValueError) as ve:
limits.parse_message(True, json.dumps([]))

assert ve.value.args[0] == "Batch messages are not supported at this time"
16 changes: 12 additions & 4 deletions src/middlewared/middlewared/utils/limits.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@


# WARNING: below methods must _not_ be audited. c.f. comment in parse_message() below
MSG_SIZE_EXTENDED_METHODS = set((
MSG_SIZE_EXTENDED_METHODS = frozenset(
'filesystem.file_receive',
))
)


class MsgSizeLimit(enum.IntEnum):
Expand Down Expand Up @@ -39,7 +39,7 @@ def __str__(self):

def parse_message(authenticated: bool, msg_data: str) -> dict:
"""
Check given message to determine whether it exceeds size limits
Parses the JSON message and ensures that it is a dict, and it does not exceed size limits.

WARNING: RFC5424 (syslog) specifies that SDATA of message should never
exceed 64 KiB. The default syslog-ng configuration will not parse messages
Expand Down Expand Up @@ -71,7 +71,15 @@ def parse_message(authenticated: bool, msg_data: str) -> dict:

message = ejson.loads(msg_data)

if (method := message.get('method')) in MSG_SIZE_EXTENDED_METHODS:
try:
method = message.get('method')
except Exception:
if isinstance(message, list):
raise ValueError('Batch messages are not supported at this time')

raise ValueError('Invalid Message Format')

if method in MSG_SIZE_EXTENDED_METHODS:
return message

if datalen > MsgSizeLimit.AUTHENTICATED:
Expand Down
Loading