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-280 \u2014 Improper Handling of Insufficient Permissions or Privileges 
The product does not handle or incorrectly handles when it has insufficient privileges to access resources or functionality as specified by their permissions. This may cause it to follow unexpected code paths that may leave the product in an invalid state.
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)