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

@@ -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

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).

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 [<dir>/ | <path>] 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 $