Skip to content

[FIX] User Impersonation through sendMessage API#20391

Merged
sampaiodiego merged 10 commits intodevelopfrom
api/message-impersonate
Apr 30, 2021
Merged

[FIX] User Impersonation through sendMessage API#20391
sampaiodiego merged 10 commits intodevelopfrom
api/message-impersonate

Conversation

@lucassartor
Copy link
Contributor

Proposed changes (including videos or screenshots)

Create a new permission: message-impersonate. For new installs only bot role will have the permission and for updating installs the permission will also be given to user role, so it won't break running deployments.

If a message is being sent with avatar or alias properties, it validates if the sender has the message-impersonate permission, if not, an error is throwed:

{
    "success": false,
    "error": "Not enough permission",
    "stack": "Error: Not enough permission\n ..."
}

Issue(s)

Steps to test or reproduce

Further comments

The Meteor call sendMessage allows usage of custom avatar and alias, which in combination allows impersonation of other chat room members. That way, spoofed message senders can potentially be used in social engineering attacks.

With the message-impersonate permission, that problem can be solved.

@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2021

This pull request introduces 1 alert when merging 940e5a7 into 3901dce - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@sampaiodiego sampaiodiego changed the title [FIX] Fix User Impersonation through sendMessage options [FIX] User Impersonation through sendMessage API Apr 30, 2021
@sampaiodiego sampaiodiego merged commit ee19c6b into develop Apr 30, 2021
@sampaiodiego sampaiodiego deleted the api/message-impersonate branch April 30, 2021 18:10
@sampaiodiego sampaiodiego mentioned this pull request May 28, 2021
@stefan-it
Copy link

I don't like this change, because you actually see the real user handle after the set alias.

And this change is not properly documented - the exact RocketChat version number for this BREAKING change (it is not even mentioned that this is a breaking change) is missing here:

https://developer.rocket.chat/api/rest-api/endpoints/chat/sendmessage#important

in the changelog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments