From 2abd48450b734a44f2ec5ea4a7f5f8585080d84a Mon Sep 17 00:00:00 2001 From: "Evgeny @ SimpleX Chat" <259188159+evgeny-simplex@users.noreply.github.com> Date: Sat, 7 Nov 2026 16:19:02 +0000 Subject: [PATCH] core: accept group file chunks only from the member who sent the file (#8057) * core: accept group file chunks only from the member who sent the file * core: inline group file chunk member filter in query --- src/Simplex/Chat/Library/Subscriber.hs | 4 +-- src/Simplex/Chat/Store/Files.hs | 18 +++++------ tests/ChatTests/Groups.hs | 42 ++++++++++++++++++++++++-- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index 777838eb97..36df91026e 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -2608,8 +2608,8 @@ processAgentMessageConn cxt user@User {userId} entity gks_ corrId agentConnId ag receiveInlineChunk ft chunk meta -- A group BFileChunk is a normal inline file chunk or a roster blob chunk, both located by - -- (group_id, shared_msg_id). A chunk matching no in-flight transfer (an orphaned re-served roster - -- chunk, or a missing normal file) is ignored; the outer withAckMessage acks it. + -- (group_id, shared_msg_id, sending member). A chunk matching no in-flight transfer (an orphaned re-served + -- roster chunk, or a missing or another member's normal file) is ignored; the outer withAckMessage acks it. bFileChunkGroup :: GroupInfoKeys -> GroupMember -> SharedMsgId -> FileChunk -> MsgMeta -> CM () bFileChunkGroup gInfo@(GIK GroupInfo {groupId} _) fromMember sharedMsgId chunk meta = do fileId_ <- withStore' $ \db -> getGroupRcvFileId db userId groupId (groupMemberId' fromMember) sharedMsgId diff --git a/src/Simplex/Chat/Store/Files.hs b/src/Simplex/Chat/Store/Files.hs index 1ba7c98fce..a923657fa6 100644 --- a/src/Simplex/Chat/Store/Files.hs +++ b/src/Simplex/Chat/Store/Files.hs @@ -355,21 +355,21 @@ getGroupFileIdBySharedMsgId db userId groupId sharedMsgId = (userId, groupId, sharedMsgId) -- Resolve the in-flight received group inline file for a chunk: read its file_type by shared_msg_id --- (LIMIT 1 is safe -- all files sharing a shared_msg_id share a type), then look up by type: a roster --- file is scoped to its source relay (every relay re-serves the owner's same shared_msg_id, so the source --- disambiguates), a normal file is by shared_msg_id. Nothing => no in-flight transfer (orphaned chunk). +-- (LIMIT 1 is safe -- all files sharing a shared_msg_id share a type), then look up by type, in both +-- cases scoped to the member the chunk came from: a roster file to its source relay (every relay +-- re-serves the owner's same shared_msg_id, so the source disambiguates), a normal file to the member +-- who sent it (shared_msg_id is known to all members, so only the sender can complete or cancel it). +-- Nothing => no in-flight transfer from this member (orphaned chunk, or a chunk for another member's file). getGroupRcvFileId :: DB.Connection -> UserId -> Int64 -> GroupMemberId -> SharedMsgId -> IO (Maybe Int64) getGroupRcvFileId db userId groupId fromMemberId sharedMsgId = do fileType_ <- getFileType case fileType_ of - Just FTRoster -> + Just fileType -> maybeFirstRow fromOnly $ - DB.query db (rcvFileIdQ <> " AND r.group_member_id = ?") (userId, groupId, sharedMsgId, FTRoster, fromMemberId) - Just FTNormal -> - maybeFirstRow fromOnly $ - DB.query db rcvFileIdQ (userId, groupId, sharedMsgId, FTNormal) + DB.query db rcvFileIdQ (userId, groupId, sharedMsgId, fileType, fromMemberId) Nothing -> pure Nothing where + getFileType :: IO (Maybe FileType) getFileType = maybeFirstRow fromOnly $ DB.query db "SELECT file_type FROM files WHERE user_id = ? AND group_id = ? AND shared_msg_id = ? LIMIT 1" (userId, groupId, sharedMsgId) @@ -377,7 +377,7 @@ getGroupRcvFileId db userId groupId fromMemberId sharedMsgId = do [sql| SELECT f.file_id FROM files f JOIN rcv_files r ON r.file_id = f.file_id - WHERE f.user_id = ? AND f.group_id = ? AND f.shared_msg_id = ? AND f.file_type = ? + WHERE f.user_id = ? AND f.group_id = ? AND f.shared_msg_id = ? AND f.file_type = ? AND r.group_member_id = ? |] -- The roster scratch file for a transfer (for fs/handle cleanup before deleting the transfer). diff --git a/tests/ChatTests/Groups.hs b/tests/ChatTests/Groups.hs index 4c690dad8d..a8192ef77a 100644 --- a/tests/ChatTests/Groups.hs +++ b/tests/ChatTests/Groups.hs @@ -33,13 +33,13 @@ import qualified Data.Map.Strict as M import qualified Data.Text as T import Simplex.Chat.Badges (FileSizeLimits (..)) import Simplex.Chat.Controller (ChatController (ChatController, smpAgent, subscriptionMode), ChatConfig (..), ChatEvent (CEvtDuplicateGroupMessage), ChatHooks (..), ChatLogLevel (..), defaultChatHooks, withAgent) -import Simplex.Chat.Library.Internal (encodeConnInfo, getShortLinkConnReq, uniqueMsgMentions, updatedMentionNames) +import Simplex.Chat.Library.Internal (encodeConnInfo, getShortLinkConnReq, sendDirectMemberMessage, uniqueMsgMentions, updatedMentionNames) import Simplex.Chat.Markdown (parseMaybeMarkdownList) import Simplex.Chat.Messages (CIMention (..), CIMentionMember (..), ChatItemId) import Simplex.Chat.Messages.Batch (encodeBinaryBatch, encodeFwdElement) import Simplex.Chat.Messages.CIContent (publicGroupNoE2EText) import Simplex.Chat.Options -import Simplex.Chat.Protocol (ChatMessage (ChatMessage), ChatMsgEvent (XFile, XGrpMemFwd, XGrpMemIntro, XGrpMemNew, XGrpRelayInv, XInfo, XMember, XMsgUpdate, XMsgNew, XMsgDel), FwdSender (FwdMember, FwdChannel), GrpMsgForward (GrpMsgForward), MsgContainer (..), MsgMention (..), MsgContent (..), VerifiedMsg (VMUnsigned), chatMsgToBody, mcSimple, msgContentText, supportedChatVRange) +import Simplex.Chat.Protocol (ChatMessage (ChatMessage), ChatMsgEvent (BFileChunk, XFile, XGrpMemFwd, XGrpMemIntro, XGrpMemNew, XGrpRelayInv, XInfo, XMember, XMsgUpdate, XMsgNew, XMsgDel), FileChunk (..), FwdSender (FwdMember, FwdChannel), GrpMsgForward (GrpMsgForward), MsgContainer (..), MsgMention (..), MsgContent (..), VerifiedMsg (VMUnsigned), chatMsgToBody, mcSimple, msgContentText, supportedChatVRange) import Simplex.Chat.Types import Simplex.Chat.Types.MemberRelations (MemberRelation (..), getRelation, setRelation) import Simplex.Chat.Types.Shared (GroupMemberRole (..), GroupAcceptance (..)) @@ -113,6 +113,7 @@ chatGroupTests = do it "remove member with messages (full deletion is enabled)" testDeleteMemberWithMessages it "remove member with messages mark deleted" testDeleteMemberMarkMessagesDeleted it "remove member - delete messages of left/removed members" testDeleteMemberMessagesLeftRemoved + it "file chunks from a member who did not send the file are ignored" testGroupFileChunkOtherMember describe "batch send messages" $ do it "send multiple messages api" testSendMulti it "send multiple timed messages" testSendMultiTimed @@ -2279,6 +2280,43 @@ testDeleteMemberMessagesLeftRemoved = cath #$> ("/_get chat #1 count=3", chat, [(1, "1"), (0, "2"), (1, "left (signed)")]) dan #$> ("/_get chat #1 count=4", chat, [(0, "1"), (1, "2"), (0, "left (signed)"), (0, "removed you (signed)")]) +testGroupFileChunkOtherMember :: HasCallStack => TestParams -> IO () +testGroupFileChunkOtherMember = + testChat3 aliceProfile bobProfile cathProfile $ + \alice bob cath -> do + createGroup3 "team" alice bob cath + + -- the XFTP server is not started yet, so alice's file stays uploading while cath sends her chunks + alice #> "/f #team ./tests/fixtures/test.txt" + alice <## "use /fc 1 to cancel sending" + forM_ [bob, cath] $ \cc -> do + cc <# "#team alice> sends file test.txt (11 bytes / 11 bytes)" + cc <## "use /fr 1 [/ | ] to receive it" + + -- cath, with a modified client, sends bob chunks naming alice's file over her own member connection + sharedMsgId <- itemSharedMsgId cath + GroupMember {groupId, activeConn = Just conn} <- getTestMember cath "team" "bob" + let cathSendsChunk chunk = do + runExceptT (sendDirectMemberMessage conn (BFileChunk sharedMsgId chunk) groupId) `runReaderT` chatController cath + >>= either (fail . show) (\_ -> pure ()) + -- the next message on the same connection is shown only after the chunk is processed + cath #> "#team hi" + [alice, bob] *<# "#team cath> hi" + + -- the cancel chunk is ignored, bob can still receive the file + cathSendsChunk FileChunkCancel + bob ##> ("/fr 1 " <> tmpDir bob) + bob <## ("saving file 1 from alice to " <> tmpFile bob "test.txt") + -- the data chunk is ignored, the file is not completed with cath's content + cathSendsChunk $ FileChunk 1 "cath's file" + + withXFTPServer alice $ do + alice <## "completed uploading file 1 (test.txt) for #team" + bob <## "started receiving file 1 (test.txt) from alice" + bob <## "completed receiving file 1 (test.txt) from alice" + src <- B.readFile "./tests/fixtures/test.txt" + B.readFile (tmpFile bob "test.txt") `shouldReturn` src + testSendMulti :: HasCallStack => TestParams -> IO () testSendMulti = testChat3 aliceProfile bobProfile cathProfile $