mirror of
https://github.com/simplex-chat/simplex-chat.git
synced 2026-10-20 12:22:23 +00:00
review fixes
- core: reject service payloads nested deeper than 32 levels, so a hostile service cannot crash the app's JSON decoder - core: one parseServiceBody (decompress, decode, depth check) at all three decode points: incoming requests, sendServiceRequestTo, redeemBadgeCode - directory: '%' and '_' in search text match literally instead of acting as LIKE wildcards - directory: search text over 100 characters is rejected over RPC - directory: a new directory creates its address with DR keys, so service requests work without a manual key rotation - directory: service requests are always processed; the --service-requests switch is no longer needed for the directory - directory: the in-flight service request cap is a DirectoryOpts field (default 8) instead of a hardcoded constant - directory: the page-fit check compresses the exact response object that is sent, not a proxy with a different key order and cursor - directory: search entries omit the full link when a short link exists, saving hundreds of bytes per entry in the envelope - ios: add DirectorySearch.swift and DirectorySearchView.swift to the Xcode project; the app did not build without them - ios: in the onboarding state the search bar is at the bottom in one-hand mode, as in the chat list - ios: in the onboarding state a failed search shows the retry row instead of returning to the cards - ios: a search with no results shows a "No results" row - ios: the "No chats found" overlay no longer covers directory results, the retry row or the no-results row - ios: directory rows are rendered by one builder for both the onboarding and chat-list branches - ios: retrying after a failed "Show more" resumes from the cursor instead of restarting the search - ios: cancelling the spinner, or tapping a result mid-search, drops the pending request without clearing results already shown - ios: typing during a search stops the spinner, and a stale response no longer clears a newer search's spinner - ios: repeated Search taps for the same text while a search is in flight are ignored - ios: channel results show the subscriber count and channel icon; group results show the group icon - ios: remove the unused searchText parameter of SearchInDirectoryRow - android: a directory search timeout no longer pops the retry alert (the UI will show a retry row) - tests: RPC paging by the echoed cursor, literal wildcards and the length bound, rejection over the cap, page fitting to the envelope, nested payload rejection
This commit is contained in:
@@ -1500,9 +1500,7 @@ processChatCommand cxt nm = \case
|
||||
pure $ CRServiceReplyAccepted user (AgentConnId connId)
|
||||
APIRejectServiceRequest userId requestId reason -> withUserId userId $ \user -> do
|
||||
let AgentInvId invId = requestId
|
||||
-- A reason is required for the requester to fail fast; without it the request is dropped
|
||||
-- silently and the caller waits out its timeout. Async, so a service shedding load does
|
||||
-- not block on a network round trip per rejected request.
|
||||
-- without a reason the request is dropped silently and the requester waits out its timeout
|
||||
withAgent $ \a -> rejectServiceRequestAsync a "" (aUserId user) invId (encodeUtf8 <$> reason)
|
||||
ok user
|
||||
APISendCallInvitation contactId callType -> withUser $ \user -> do
|
||||
@@ -5245,7 +5243,7 @@ redeemBadgeCode nm user@User {userId} codeText = do
|
||||
maybe (withStore' $ \db -> createBadgeCodeRedemption db g user codeSent now) pure redemption_
|
||||
let req = BadgeServiceRequest {version = currentBadgeServiceVersion, purchaseKey = Just purchaseKey, request = BSCRedeemBadgeCode {masterKey, code = codeSent}}
|
||||
respBytes <- sendServiceRequestBytes nm user sendTarget Nothing (Just purchasePrivKey) req
|
||||
respData <- either (const $ throwRedeemError $ BREInvalidResponse "not JSON") pure $ J.eitherDecodeStrict' respBytes
|
||||
respData <- either (const $ throwRedeemError $ BREInvalidResponse "not JSON") pure $ parseServiceBody respBytes
|
||||
case J.fromJSON (J.Object respData) of
|
||||
J.Error _ -> throwRedeemError $ BREInvalidResponse "not a badge service response"
|
||||
J.Success BSPError {code = errCode} -> do
|
||||
@@ -5640,14 +5638,13 @@ applyBadgeStatement db g purchaseId badgeType BadgeStatement {entries} cred_ now
|
||||
sendServiceRequestTo :: J.ToJSON a => NetworkRequestMode -> User -> ConnectTarget 'CMContact -> Maybe NominalDiffTime -> Maybe C.PrivateKeyEd25519 -> a -> CM J.Object
|
||||
sendServiceRequestTo nm user sendTarget requestTimeout signKey request =
|
||||
sendServiceRequestBytes nm user sendTarget requestTimeout signKey request
|
||||
>>= either (const $ throwCmdError "invalid service response") pure . J.eitherDecodeStrict'
|
||||
>>= either (const $ throwCmdError "invalid service response") pure . parseServiceBody
|
||||
|
||||
sendServiceRequestBytes :: J.ToJSON a => NetworkRequestMode -> User -> ConnectTarget 'CMContact -> Maybe NominalDiffTime -> Maybe C.PrivateKeyEd25519 -> a -> CM ByteString
|
||||
sendServiceRequestBytes nm user sendTarget requestTimeout signKey request = do
|
||||
cReq <- resolveServiceTarget sendTarget
|
||||
reqData <- either throwCmdError pure $ compressServiceBody $ LB.toStrict $ J.encode request
|
||||
respData <- withAgent $ \a -> sendServiceRequestAsync a (aUserId user) cReq requestTimeout signKey reqData
|
||||
either (const $ throwCmdError "invalid service response") pure $ decompressServiceBody respData
|
||||
withAgent $ \a -> sendServiceRequestAsync a (aUserId user) cReq requestTimeout signKey reqData
|
||||
where
|
||||
resolveServiceTarget = \case
|
||||
CTFullContact cReq -> pure cReq
|
||||
|
||||
@@ -108,7 +108,6 @@ import Text.Read (readMaybe)
|
||||
import UnliftIO.Concurrent (ThreadId, forkIO, mkWeakThreadId)
|
||||
import UnliftIO.Directory
|
||||
import UnliftIO.STM
|
||||
import qualified Data.Aeson as J
|
||||
|
||||
smallGroupsRcptsMemLimit :: Int
|
||||
smallGroupsRcptsMemLimit = 20
|
||||
@@ -1396,7 +1395,7 @@ processAgentMessageConn cxt user@User {userId} entity corrId agentConnId agentMe
|
||||
_ -> pure ()
|
||||
SREQ invId sigKey_ payload ->
|
||||
chatReadVar processServiceRequests >>= \case
|
||||
True -> case J.eitherDecodeStrict' =<< decompressServiceBody payload of
|
||||
True -> case parseServiceBody payload of
|
||||
Right request -> toView $ CEvtServiceRequest user (AgentInvId invId) sigKey_ request
|
||||
Left e -> logError ("service request dropped, invalid payload: " <> tshow e) >> dropSReq
|
||||
-- the requester gets no reply and waits out its timeout, so this must be visible
|
||||
|
||||
@@ -315,7 +315,6 @@ coreChatOptsP appDir defaultDbName = do
|
||||
( long "ha"
|
||||
<> help "Run as a highly available client (this may increase traffic in groups)"
|
||||
)
|
||||
-- TODO [directory] default this on for the directory binary, so a deployment cannot omit it
|
||||
serviceRequests <-
|
||||
switch
|
||||
( long "service-requests"
|
||||
|
||||
@@ -1063,6 +1063,22 @@ decompressServiceBody body = case B.uncons body of
|
||||
Right _ -> Left "unexpected compressed batch"
|
||||
_ -> Right body
|
||||
|
||||
-- The apps decode a service payload recursively on a fixed stack, and no service nests deeper
|
||||
-- than a few levels, so depth is bounded here rather than left to each client.
|
||||
maxServiceBodyDepth :: Int
|
||||
maxServiceBodyDepth = 32
|
||||
|
||||
parseServiceBody :: ByteString -> Either String J.Object
|
||||
parseServiceBody body = do
|
||||
o <- J.eitherDecodeStrict' =<< decompressServiceBody body
|
||||
when (depth (J.Object o) > maxServiceBodyDepth) $ Left "service payload is nested too deeply"
|
||||
pure o
|
||||
where
|
||||
depth = \case
|
||||
J.Object kv -> 1 + foldr (max . depth) 0 kv
|
||||
J.Array vs -> 1 + foldr (max . depth) 0 vs
|
||||
_ -> 0
|
||||
|
||||
justTrue :: Bool -> Maybe Bool
|
||||
justTrue True = Just True
|
||||
justTrue False = Nothing
|
||||
|
||||
Reference in New Issue
Block a user