From e863ee4c2732a856250720448379bb8054ff8091 Mon Sep 17 00:00:00 2001 From: Evgeny Date: Tue, 29 Sep 2026 21:56:03 +0100 Subject: [PATCH] core: limit depth of forwarded messages (#7614) Co-authored-by: Evgeny @ SimpleX Chat <259188159+evgeny-simplex@users.noreply.github.com> --- docs/protocol/simplex-chat.md | 2 +- src/Simplex/Chat/Protocol.hs | 12 ++++++++---- tests/ProtocolTests.hs | 25 +++++++++++++++++++------ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/docs/protocol/simplex-chat.md b/docs/protocol/simplex-chat.md index 040064864c..45344d49e0 100644 --- a/docs/protocol/simplex-chat.md +++ b/docs/protocol/simplex-chat.md @@ -264,7 +264,7 @@ Currently members can have one of four roles - `owner`, `admin`, `member` and `o `x.grp.direct.inv` message is sent to a group member to propose establishing a direct connection between members, thus creating a contact with another member. -`x.grp.msg.forward` message is sent by inviting member to forward messages between introduced members, while they are connecting. +`x.grp.msg.forward` message is sent by inviting member to forward messages between introduced members, while they are connecting. This message MUST NOT contain another `x.grp.msg.forward` message. ### Channels: relay-mediated groups diff --git a/src/Simplex/Chat/Protocol.hs b/src/Simplex/Chat/Protocol.hs index fbdafca782..5f920f8906 100644 --- a/src/Simplex/Chat/Protocol.hs +++ b/src/Simplex/Chat/Protocol.hs @@ -935,6 +935,9 @@ maxDecompressedMsgLength = 65536 maxBatchElementCount :: Int maxBatchElementCount = 255 +maxFwdDepth :: Int +maxFwdDepth = 1 + -- Defensive entry-count bound for the roster blob parser (rosterBlobP) and the -- promotion cap over the promoted (member/moderator/admin) set. maxGroupRosterSize :: Int @@ -1387,8 +1390,8 @@ appBinaryToCM AppMessageBinary {msgId, tag, body} = do msg = \case BFileChunk_ -> BFileChunk <$> (SharedMsgId <$> smpP) <*> (unIFC <$> smpP) -appJsonToCM :: AppMessageJson -> Either String (ChatMessage 'Json) -appJsonToCM AppMessageJson {v, msgId, event, params} = do +appJsonToCM :: Int -> AppMessageJson -> Either String (ChatMessage 'Json) +appJsonToCM fwdDepth AppMessageJson {v, msgId, event, params} = do eventTag <- strDecode $ encodeUtf8 event chatMsgEvent <- msg eventTag pure ChatMessage {chatVRange = maybe chatInitialVRange fromChatVRange v, msgId, chatMsgEvent} @@ -1463,11 +1466,12 @@ appJsonToCM AppMessageJson {v, msgId, event, params} = do XGrpRosterAck_ -> XGrpRosterAck <$> p "version" <*> opt "error" XGrpRosterRequest_ -> XGrpRosterRequest <$> opt "version" XGrpMsgForward_ -> do + when (fwdDepth >= maxFwdDepth) $ Left "forward depth exceeds limit" fwdSender <- opt "memberId" >>= \case Just memberId -> FwdMember memberId . fromMaybe "" <$> opt "memberName" Nothing -> pure FwdChannel fwdBrokerTs <- p "msgTs" - XGrpMsgForward (GrpMsgForward {fwdSender, fwdBrokerTs}) <$> p "msg" + XGrpMsgForward (GrpMsgForward {fwdSender, fwdBrokerTs}) <$> (appJsonToCM (fwdDepth + 1) =<< p "msg") XInfoProbe_ -> XInfoProbe <$> p "probe" XInfoProbeCheck_ -> XInfoProbeCheck <$> p "probeHash" XInfoProbeOk_ -> XInfoProbeOk <$> p "probe" @@ -1580,7 +1584,7 @@ instance ToJSON (ChatMessage 'Json) where toJSON = (\(AMJson msg) -> toJSON msg) . chatToAppMessage instance FromJSON (ChatMessage 'Json) where - parseJSON v = appJsonToCM <$?> parseJSON v + parseJSON v = appJsonToCM 0 <$?> parseJSON v instance FromField (ChatMessage 'Json) where fromField = blobFieldDecoder J.eitherDecodeStrict' diff --git a/tests/ProtocolTests.hs b/tests/ProtocolTests.hs index d748b5cba2..0c166fe71c 100644 --- a/tests/ProtocolTests.hs +++ b/tests/ProtocolTests.hs @@ -35,6 +35,7 @@ protocolTests = do decodeChatMessageTest shortLinkDataTests batchLimitTests + forwardDepthTests preferencesJSONTests preferencesJSONTests :: Spec @@ -86,6 +87,19 @@ batchLimitTests = describe "Chat message batch limits" $ do [Left e] -> e rs -> "expected a single error, got " <> show (length rs) <> " results" +forwardDepthTests :: Spec +forwardDepthTests = describe "Chat message forward depth limit" $ do + it "parses x.grp.msg.forward at the depth limit" $ + chatMsgToBody (nestedFwd maxFwdDepth) ==## nestedFwd maxFwdDepth + it "rejects x.grp.msg.forward above the depth limit" $ + case parseChatMessages $ chatMsgToBody $ nestedFwd $ maxFwdDepth + 1 of + [Left e] -> e `shouldSatisfy` isInfixOf "forward depth exceeds limit" + _ -> expectationFailure "single parse error expected" + where + nestedFwd :: Int -> ChatMessage 'Json + nestedFwd 0 = ChatMessage chatInitialVRange Nothing $ XMsgNew $ mcSimple $ MCText "hello" + nestedFwd n = ChatMessage chatInitialVRange Nothing $ XGrpMsgForward (GrpMsgForward FwdChannel $ systemToUTCTime $ MkSystemTime 1 1) (nestedFwd $ n - 1) + srv :: SMPServer srv = SMPServer "smp.simplex.im" "5223" (C.KeyHash "\215m\248\251") @@ -389,12 +403,11 @@ decodeChatMessageTest = describe "Chat message encoding/decoding" $ do it "x.grp.direct.inv without content" $ "{\"v\":\"9\",\"event\":\"x.grp.direct.inv\",\"params\":{\"connReq\":\"simplex:/invitation#/?v=1&smp=smp%3A%2F%2F1234-w%3D%3D%40smp.simplex.im%3A5223%2F3456-w%3D%3D%23%2F%3Fv%3D1-4%26dh%3DMCowBQYDK2VuAyEAjiswwI3O_NlS8Fk3HJUW870EY2bAwmttMBsvRB9eV3o%253D&e2e=v%3D3%26x3dh%3DMEIwBQYDK2VvAzkAmKuSYeQ_m0SixPDS8Wq8VBaTS1cW-Lp0n0h4Diu-kUpR-qXx4SDJ32YGEFoGFGSbGPry5Ychr6U%3D%2CMEIwBQYDK2VvAzkAmKuSYeQ_m0SixPDS8Wq8VBaTS1cW-Lp0n0h4Diu-kUpR-qXx4SDJ32YGEFoGFGSbGPry5Ychr6U%3D\"}}" #==# XGrpDirectInv testConnReq Nothing Nothing - -- it "x.grp.msg.forward" - -- $ "{\"v\":\"9\",\"event\":\"x.grp.msg.forward\",\"params\":{\"msgForward\":{\"memberId\":\"AQIDBA==\",\"msg\":\"{\"v\":\"9\",\"event\":\"x.msg.new\",\"params\":{\"content\":{\"text\":\"hello\",\"type\":\"text\"}}}\",\"msgTs\":\"1970-01-01T00:00:01.000000001Z\"}}}" - -- #==# XGrpMsgForward - -- (MemberId "\1\2\3\4") - -- (ChatMessage chatInitialVRange (Just $ SharedMsgId "\1\2\3\4") (XMsgNew (mcSimple (MCText "hello")))) - -- (systemToUTCTime $ MkSystemTime 1 1) + it "x.grp.msg.forward" $ + "{\"v\":\"9\",\"event\":\"x.grp.msg.forward\",\"params\":{\"memberId\":\"AQIDBA==\",\"memberName\":\"alice\",\"msg\":{\"v\":\"9\",\"msgId\":\"AQIDBA==\",\"event\":\"x.msg.new\",\"params\":{\"content\":{\"text\":\"hello\",\"type\":\"text\"}}},\"msgTs\":\"1970-01-01T00:00:01.000000001Z\"}}" + #==# XGrpMsgForward + (GrpMsgForward (FwdMember (MemberId "\1\2\3\4") "alice") (systemToUTCTime $ MkSystemTime 1 1)) + (ChatMessage chatInitialVRange (Just $ SharedMsgId "\1\2\3\4") (XMsgNew (mcSimple (MCText "hello")))) it "x.info.probe" $ "{\"v\":\"9\",\"event\":\"x.info.probe\",\"params\":{\"probe\":\"AQIDBA==\"}}" #==# XInfoProbe (Probe "\1\2\3\4")