CVE-2023-22737 (Other, published 2023-01-28T00:15Z)
Description: value: wire-server provides back end services for Wire, a team communication and collaboration platform. Prior to version 2022-12-09, every member of a Conversation can remove a Bot from a Conversation due to a missing permissions check. Only Conversation admins should be able to remove Bots. Regular Conversations are not allowed to do so. The issue is fixed in wire-server 2022-12-09 and is already deployed on all Wire managed services. On-premise instances of wire-server need to be updated to 2022-12-09/Chart 4.29.0, so that their backends are no longer affected. There are no known workarounds.
Severity: nan (CVSS v3 base score: 6.5)
Weakness: CWE-862 \u2014 Missing Authorization
The product does not perform an authorization check when an actor attempts to access a resource or perform an action.
Repository: https://github.com/wireapp/wire-server
Files changed:
changelog.d/3-bug-fixes/pr-2870
services/brig/src/Brig/Provider/API.hs
services/brig/test/integration/API/Provider.hs
services/brig/test/integration/API/Team/Util.hs
services/galley/src/Galley/API/Update.hs
Commit message: [SQSERVICES-1801] Prevent dead bots in database (#2870)


Vulnerable code:
createTeamConv g tid u us mtimer = do
        NewConv us [] Nothing (Set.fromList []) Nothing tinfo mtimer Nothing roleNameWireAdmin ProtocolProteusTag Nothing
       WaiRoutes
       MemberStore
  let lcnv = qualifyAs lusr (Data.convId c)
  let (bots, users) = localBotsAndUsers (Data.convLocalMembers c)

Fixed code:
Prevention of storing unnecessary data in the database if adding a bot to a conversation fails.
  -- Check that the user is a conversation admin and therefore is allowed to add a bot to this conversation.
  -- Note that this precondition is also checked in the internal galley API,
  -- but by having this check here we prevent any (useless) data to be written to the database
  -- as well as the unnecessary creation of the bot via the external service API call.
  -- However, in case we refine the roles model in the future, this check might not be granular enough.
  -- In that case we should rather do an internal call to galley to check for the correct permissions.
  -- Also see `removeBot` for a similar check.
  guardConvAdmin cnv
  -- Check that the user is a conversation admin and therefore is allowed to remove a bot from the conversation.
  -- Note that this precondition is also checked in the internal galley API.
  -- However, in case we refine the roles model in the future, this check might not be granular enough.
  -- In that case we should rather do an internal call to galley to check for the correct permissions.
  -- Also see `addBot` for a similar check.
  guardConvAdmin cnv
guardConvAdmin :: Conversation -> ExceptT Error (AppT r) ()
guardConvAdmin conv = do
  let selfMember = cmSelf . cnvMembers $ conv
  unless (memConvRoleName selfMember == roleNameWireAdmin) $ throwStd accessDenied

            test p "add-remove-access-denied-for-non-conv-admin" $ testNonConvAdminCannotAddRemoveBot conf db b g,
testNonConvAdminCannotAddRemoveBot :: Config -> DB.ClientState -> Brig -> Galley -> Http ()
testNonConvAdminCannotAddRemoveBot config db brig galley = withTestService config db brig defServiceApp $ \sref _buf -> do
  let pid = sref ^. serviceRefProvider
  let sid = sref ^. serviceRefId
  (ownerId, tid) <- Team.createUserWithTeam brig
  member <- Team.createTeamMember brig galley ownerId tid fullPermissions
  let memberId = userId member
  whitelistService brig ownerId tid pid sid
  cid <- Team.createTeamConvWithRole roleNameWireMember galley tid ownerId [memberId] Nothing
  addBot brig memberId pid sid cid !!! do
    const 403 === statusCode
    const (Just "access-denied") === fmap Error.label . responseJsonMaybe
  rs <- responseJsonError =<< addBot brig ownerId pid sid cid <!! const 201 === statusCode
  let bid = rsAddBotId rs
      buid = botUserId bid
  getUser brig ownerId buid !!! const 200 === statusCode
  removeBot brig memberId cid bid !!! do
    const 403 === statusCode
    const (Just "access-denied") === fmap Error.label . responseJsonMaybe
  -- also check the internal galley API
  removeBotInternal galley memberId cid bid !!! do
    const 403 === statusCode
    const (Just "action-denied") === fmap Error.label . responseJsonMaybe

data RemoveBot = RemoveBot
  { _rmBotConv :: !ConvId,
    _rmBotId :: !BotId
  }

instance ToJSON RemoveBot where
  toJSON a =
    object
      [ "conversation" .= _rmBotConv a,
        "bot" .= _rmBotId a
      ]

removeBotInternal ::
  Galley ->
  UserId ->
  ConvId ->
  BotId ->
  Http ResponseLBS
removeBotInternal galley uid cid bid =
  delete $
    galley
      . paths ["i", "bots"]
      . header "Z-User" (toByteString' uid)
      . Bilge.json (RemoveBot cid bid)

createTeamConv = createTeamConvWithRole roleNameWireAdmin

createTeamConvWithRole :: HasCallStack => RoleName -> Galley -> TeamId -> UserId -> [UserId] -> Maybe Milliseconds -> Http ConvId
createTeamConvWithRole role g tid u us mtimer = do
        NewConv
          us
          []
          Nothing
          (Set.fromList [])
          Nothing
          tinfo
          mtimer
          Nothing
          role
          ProtocolProteusTag
          Nothing
      -- Note that in brig from where this internal handler is called, we additionally check for conversation admin role.
      -- Remember to change this if we ever want to allow non admins to add bots.
       WaiRoutes,
       ErrorS ('ActionDenied 'RemoveConversationMember)
       MemberStore,
       ErrorS ('ActionDenied 'RemoveConversationMember)
  let (bots, users) = localBotsAndUsers (Data.convLocalMembers c)
  -- A bot can remove itself (which will internally be triggered when a service is deleted),
  -- otherwise we have to check for the correct permissions
  unless (botUserId (b ^. rmBotId) == tUnqualified lusr) $ do
    -- Note that in brig from where this internal handler is called, we additionally check for conversation admin role.
    -- Remember to change this if we ever want to allow non admins to remove bots.
    self <- getSelfMemberFromLocals (tUnqualified lusr) users
    ensureActionAllowed SRemoveConversationMember self
  let lcnv = qualifyAs lusr (Data.convId c)