mirror of
				https://github.com/zulip/zulip.git
				synced 2025-11-03 21:43:21 +00:00 
			
		
		
		
	submessages: Add verify_submessage_sender.
Before this change a rogue actor could try to widgetize another person's message. (The rogue actor would already have access to read the message.)
This commit is contained in:
		@@ -2151,6 +2151,30 @@ def bulk_insert_ums(ums: List[UserMessageLite]) -> None:
 | 
				
			|||||||
        execute_values(cursor.cursor, query, vals)
 | 
					        execute_values(cursor.cursor, query, vals)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					def verify_submessage_sender(
 | 
				
			||||||
 | 
					    *,
 | 
				
			||||||
 | 
					    message_id: int,
 | 
				
			||||||
 | 
					    message_sender_id: int,
 | 
				
			||||||
 | 
					    submessage_sender_id: int,
 | 
				
			||||||
 | 
					) -> None:
 | 
				
			||||||
 | 
					    """Even though our submessage architecture is geared toward
 | 
				
			||||||
 | 
					    collaboration among all message readers, we still enforce
 | 
				
			||||||
 | 
					    the the first person to attach a submessage to the message
 | 
				
			||||||
 | 
					    must be the original sender of the message.
 | 
				
			||||||
 | 
					    """
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    if message_sender_id == submessage_sender_id:
 | 
				
			||||||
 | 
					        return
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    if SubMessage.objects.filter(
 | 
				
			||||||
 | 
					        message_id=message_id,
 | 
				
			||||||
 | 
					        sender_id=message_sender_id,
 | 
				
			||||||
 | 
					    ).exists():
 | 
				
			||||||
 | 
					        return
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    raise JsonableError(_("You cannot attach a submessage to this message."))
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
def do_add_submessage(
 | 
					def do_add_submessage(
 | 
				
			||||||
    realm: Realm,
 | 
					    realm: Realm,
 | 
				
			||||||
    sender_id: int,
 | 
					    sender_id: int,
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -100,6 +100,42 @@ class TestBasics(ZulipTestCase):
 | 
				
			|||||||
        result = self.client_post("/json/submessage", payload)
 | 
					        result = self.client_post("/json/submessage", payload)
 | 
				
			||||||
        self.assert_json_error(result, "Invalid message(s)")
 | 
					        self.assert_json_error(result, "Invalid message(s)")
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def test_original_sender_enforced(self) -> None:
 | 
				
			||||||
 | 
					        cordelia = self.example_user("cordelia")
 | 
				
			||||||
 | 
					        hamlet = self.example_user("hamlet")
 | 
				
			||||||
 | 
					        stream_name = "Verona"
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        message_id = self.send_stream_message(
 | 
				
			||||||
 | 
					            sender=cordelia,
 | 
				
			||||||
 | 
					            stream_name=stream_name,
 | 
				
			||||||
 | 
					        )
 | 
				
			||||||
 | 
					        self.login_user(hamlet)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        payload = dict(
 | 
				
			||||||
 | 
					            message_id=message_id,
 | 
				
			||||||
 | 
					            msg_type="whatever",
 | 
				
			||||||
 | 
					            content="{}",
 | 
				
			||||||
 | 
					        )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        # Hamlet can't just go attaching submessages to Cordelia's
 | 
				
			||||||
 | 
					        # message, even though he does have read access here to the
 | 
				
			||||||
 | 
					        # message itself.
 | 
				
			||||||
 | 
					        result = self.client_post("/json/submessage", payload)
 | 
				
			||||||
 | 
					        self.assert_json_error(result, "You cannot attach a submessage to this message.")
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        # Since Hamlet is actually subcribed to the stream, he is welcome
 | 
				
			||||||
 | 
					        # to send submessages to Cordelia once she initiates the "subconversation".
 | 
				
			||||||
 | 
					        do_add_submessage(
 | 
				
			||||||
 | 
					            realm=cordelia.realm,
 | 
				
			||||||
 | 
					            sender_id=cordelia.id,
 | 
				
			||||||
 | 
					            message_id=message_id,
 | 
				
			||||||
 | 
					            msg_type="whatever",
 | 
				
			||||||
 | 
					            content="whatever",
 | 
				
			||||||
 | 
					        )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        result = self.client_post("/json/submessage", payload)
 | 
				
			||||||
 | 
					        self.assert_json_success(result)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def test_endpoint_success(self) -> None:
 | 
					    def test_endpoint_success(self) -> None:
 | 
				
			||||||
        cordelia = self.example_user("cordelia")
 | 
					        cordelia = self.example_user("cordelia")
 | 
				
			||||||
        hamlet = self.example_user("hamlet")
 | 
					        hamlet = self.example_user("hamlet")
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -4,7 +4,7 @@ from django.http import HttpRequest, HttpResponse
 | 
				
			|||||||
from django.utils.translation import gettext as _
 | 
					from django.utils.translation import gettext as _
 | 
				
			||||||
 | 
					
 | 
				
			||||||
from zerver.decorator import REQ, has_request_variables
 | 
					from zerver.decorator import REQ, has_request_variables
 | 
				
			||||||
from zerver.lib.actions import do_add_submessage
 | 
					from zerver.lib.actions import do_add_submessage, verify_submessage_sender
 | 
				
			||||||
from zerver.lib.message import access_message
 | 
					from zerver.lib.message import access_message
 | 
				
			||||||
from zerver.lib.response import json_error, json_success
 | 
					from zerver.lib.response import json_error, json_success
 | 
				
			||||||
from zerver.lib.validator import check_int
 | 
					from zerver.lib.validator import check_int
 | 
				
			||||||
@@ -23,6 +23,12 @@ def process_submessage(
 | 
				
			|||||||
) -> HttpResponse:
 | 
					) -> HttpResponse:
 | 
				
			||||||
    message, user_message = access_message(user_profile, message_id, lock_message=True)
 | 
					    message, user_message = access_message(user_profile, message_id, lock_message=True)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    verify_submessage_sender(
 | 
				
			||||||
 | 
					        message_id=message.id,
 | 
				
			||||||
 | 
					        message_sender_id=message.sender_id,
 | 
				
			||||||
 | 
					        submessage_sender_id=user_profile.id,
 | 
				
			||||||
 | 
					    )
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    try:
 | 
					    try:
 | 
				
			||||||
        orjson.loads(content)
 | 
					        orjson.loads(content)
 | 
				
			||||||
    except orjson.JSONDecodeError:
 | 
					    except orjson.JSONDecodeError:
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user