feat: voice to video escalation - #40983
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: a761ff2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (67)
WalkthroughAdds Pexip-backed voice-to-video escalation across REST APIs, media services, SIP signaling, conference persistence, session state, and VoIP UI components. It also adds escalation settings, localization, call-feature contracts, SIP aliases, automatic escalation handling, and supporting stories/tests. ChangesEscalation contracts and configuration
Conference persistence and SIP aliases
Server escalation workflow
SIP and Pexip integration
Media signaling and VoIP UI
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dmv/base-for-video-escalation #40983 +/- ##
================================================================
Coverage ? 69.15%
================================================================
Files ? 3563
Lines ? 140103
Branches ? 24889
================================================================
Hits ? 96890
Misses ? 39201
Partials ? 4012
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
5409bd9 to
741c235
Compare
2de9e16 to
9d3ef38
Compare
d44d016 to
9c1451a
Compare
1700b61 to
14dbba1
Compare
9c1451a to
4b6cb58
Compare
14dbba1 to
73f0764
Compare
4b6cb58 to
cc99363
Compare
73f0764 to
b8ad2ae
Compare
cc99363 to
196d5bf
Compare
e776d30 to
bd59374
Compare
472884e to
ea87e42
Compare
196d5bf to
bdf36d2
Compare
382e1bd to
76a157b
Compare
bdf36d2 to
3ab8953
Compare
76a157b to
4e0a746
Compare
3ab8953 to
f62e6e0
Compare
React useId format changed on develop (':r0:' -> '_r_0_') and the video
escalation modal copy was updated; refresh ConfirmVideoEscalationModal and
PopupBlockedModal snapshots.
…s and improve hangup handling from sip states
2bb686d to
a761ff2
Compare
| private async detectVoiceCallEscalation(conference: VideoConference, participantUri: string | null): Promise<boolean> { | ||
| if (!participantUri) { | ||
| return false; | ||
| } | ||
|
|
||
| const { sipAlias, mediaCallIds: linkedMediaCallIds } = conference; | ||
|
|
||
| if (!sipAlias) { | ||
| return false; | ||
| } | ||
|
|
||
| const participantSipExtension = this.getIdentificationFromAlias(participantUri); | ||
|
|
||
| if (!participantSipExtension) { | ||
| logger.debug({ msg: 'Someone connected to a Pexip Conference via SIP, but we could not identify them.' }); | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| logger.debug({ | ||
| msg: 'Pexip Participant joined via SIP', | ||
| sipAlias: conference.sipAlias, | ||
| conferenceId: conference._id, | ||
| participantSipExtension, | ||
| }); | ||
|
|
||
| const mediaCallIds = await MediaCalls.findAllNotOverByOppositeSipExtension(participantSipExtension, { projection: { _id: 1 } }) | ||
| .map(({ _id }) => _id) | ||
| .toArray(); | ||
|
|
||
| if (mediaCallIds.length !== 1) { | ||
| // Check if the user is already linked to the conference | ||
| if (linkedMediaCallIds?.length) { | ||
| if (await MediaCalls.isUserSipExtensionInCallIds(participantSipExtension, linkedMediaCallIds)) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| logger.debug({ msg: 'Could not identify the media call that the SIP Participant is connecting from', calls: mediaCallIds }); | ||
| return mediaCallIds.length > 0; | ||
| } | ||
|
|
||
| const [mediaCallId] = mediaCallIds; | ||
|
|
||
| const updateResult = await VideoConferenceModel.addMediaCallIdByConferenceId(conference._id, mediaCallId); | ||
| if (updateResult.modifiedCount) { | ||
| await MediaCall.flagAsRemotelyEscalatedByCallId(mediaCallId); | ||
| } | ||
| } catch (err) { | ||
| logger.error({ msg: 'Unexpected error handling Pexip Voice to Video Escalation', err }); | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
Pexip Conference Guest PIN Bypass via Unrelated Active Media Call Escalation
In packages/pexip/src/endpoints/serviceConfiguration.ts, the detectVoiceCallEscalation method is called during Pexip policy configuration requests (ServerConfigurationEndpoint.get) to determine if an incoming SIP participant is escalating an active voice call to a video conference. If this method returns true, the system sets canSkipPin to true, which completely removes the guest PIN requirement (guest_pin: null) for the requested conference.
However, the implementation of detectVoiceCallEscalation only checks if there is any active media call in the system for the connecting participant's SIP extension (MediaCalls.findAllNotOverByOppositeSipExtension(participantSipExtension)). It fails to verify whether that active media call has any association or relationship with the specific VideoConference being queried.
As a result, if any user with a SIP extension is currently in any active media call, an incoming SIP connection to any Pexip conference claiming to be from that SIP extension will trigger detectVoiceCallEscalation to return true. This bypasses the guest PIN requirement for that conference, allowing unauthorized users to join.
Steps to Reproduce
- Ensure the Rocket.Chat workspace has the Pexip integration enabled and configured.
- A legitimate user with a SIP extension (e.g.,
1001) must currently be in an active, unfinished media call. - An attacker connects to a private Pexip conference (e.g.,
conf-abc) via SIP, spoofing their caller ID / SIP URI to besip:1001@.... - The Pexip server sends a policy request to Rocket.Chat's
ServerConfigurationEndpoint.getwithlocal_alias: "conf-abc"andremote_alias: "sip:1001@...". - Rocket.Chat's
detectVoiceCallEscalationfinds the unrelated active call for extension1001, associates it with conferenceconf-abc, and returnstrue. - Rocket.Chat returns the service configuration to Pexip with
guest_pin: null. - The Pexip server allows the attacker (and any other guests) to join the private conference
conf-abcwithout entering a PIN.
# An attacker can trigger this by sending a mock Pexip policy request to the Rocket.Chat Pexip endpoint, spoofing the SIP extension of an active caller:
curl -X GET "http://localhost:3000/api/v1/pexip.policy" \
-H "Content-Type: application/json" \
-d '{
"local_alias": "unrelated-private-conference-id",
"protocol": "sip",
"remote_alias": "sip:ACTIVE_USER_SIP_EXTENSION@sip.domain.com"
}'Fix with AI
A security vulnerability was found by Hacktron.
File: packages/pexip/src/endpoints/serviceConfiguration.ts
Lines: 67-120
Severity: high
Vulnerability: Pexip Conference Guest PIN Bypass via Unrelated Active Media Call Escalation
Description:
In `packages/pexip/src/endpoints/serviceConfiguration.ts`, the `detectVoiceCallEscalation` method is called during Pexip policy configuration requests (`ServerConfigurationEndpoint.get`) to determine if an incoming SIP participant is escalating an active voice call to a video conference. If this method returns `true`, the system sets `canSkipPin` to `true`, which completely removes the guest PIN requirement (`guest_pin: null`) for the requested conference.
However, the implementation of `detectVoiceCallEscalation` only checks if there is any active media call in the system for the connecting participant's SIP extension (`MediaCalls.findAllNotOverByOppositeSipExtension(participantSipExtension)`). It fails to verify whether that active media call has any association or relationship with the specific `VideoConference` being queried.
As a result, if any user with a SIP extension is currently in any active media call, an incoming SIP connection to any Pexip conference claiming to be from that SIP extension will trigger `detectVoiceCallEscalation` to return `true`. This bypasses the guest PIN requirement for that conference, allowing unauthorized users to join.
Proof of Concept:
**Steps to Reproduce**
1. Ensure the Rocket.Chat workspace has the Pexip integration enabled and configured.
2. A legitimate user with a SIP extension (e.g., `1001`) must currently be in an active, unfinished media call.
3. An attacker connects to a private Pexip conference (e.g., `conf-abc`) via SIP, spoofing their caller ID / SIP URI to be `sip:1001@...`.
4. The Pexip server sends a policy request to Rocket.Chat's `ServerConfigurationEndpoint.get` with `local_alias: "conf-abc"` and `remote_alias: "sip:1001@..."`.
5. Rocket.Chat's `detectVoiceCallEscalation` finds the unrelated active call for extension `1001`, associates it with conference `conf-abc`, and returns `true`.
6. Rocket.Chat returns the service configuration to Pexip with `guest_pin: null`.
7. The Pexip server allows the attacker (and any other guests) to join the private conference `conf-abc` without entering a PIN.
```bash
# An attacker can trigger this by sending a mock Pexip policy request to the Rocket.Chat Pexip endpoint, spoofing the SIP extension of an active caller:
curl -X GET "http://localhost:3000/api/v1/pexip.policy" \
-H "Content-Type: application/json" \
-d '{
"local_alias": "unrelated-private-conference-id",
"protocol": "sip",
"remote_alias": "sip:ACTIVE_USER_SIP_EXTENSION@sip.domain.com"
}'
```
Affected Code:
private async detectVoiceCallEscalation(conference: VideoConference, participantUri: string | null): Promise<boolean> {
if (!participantUri) {
return false;
}
const { sipAlias, mediaCallIds: linkedMediaCallIds } = conference;
if (!sipAlias) {
return false;
}
const participantSipExtension = this.getIdentificationFromAlias(participantUri);
if (!participantSipExtension) {
logger.debug({ msg: 'Someone connected to a Pexip Conference via SIP, but we could not identify them.' });
return false;
}
try {
logger.debug({
msg: 'Pexip Participant joined via SIP',
sipAlias: conference.sipAlias,
conferenceId: conference._id,
participantSipExtension,
});
const mediaCallIds = await MediaCalls.findAllNotOverByOppositeSipExtension(participantSipExtension, { projection: { _id: 1 } })
.map(({ _id }) => _id)
.toArray();
if (mediaCallIds.length !== 1) {
// Check if the user is already linked to the conference
if (linkedMediaCallIds?.length) {
if (await MediaCalls.isUserSipExtensionInCallIds(participantSipExtension, linkedMediaCallIds)) {
return true;
}
}
logger.debug({ msg: 'Could not identify the media call that the SIP Participant is connecting from', calls: mediaCallIds });
return mediaCallIds.length > 0;
}
const [mediaCallId] = mediaCallIds;
const updateResult = await VideoConferenceModel.addMediaCallIdByConferenceId(conference._id, mediaCallId);
if (updateResult.modifiedCount) {
await MediaCall.flagAsRemotelyEscalatedByCallId(mediaCallId);
}
} catch (err) {
logger.error({ msg: 'Unexpected error handling Pexip Voice to Video Escalation', err });
}
return true;
}
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), !accepted_risk <reason>, or !fixed (resolved). Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
| public async escalateCall(uid: IUser['_id'], params: { callId: string }): Promise<string> { | ||
| const { callId } = params; | ||
|
|
||
| logger.debug({ msg: 'Escalating Voice Call', method: 'MediaCallService.escalateCall', uid, callId }); | ||
|
|
||
| const call = await MediaCalls.findOneById(callId); | ||
|
|
||
| try { | ||
| if (!call?.acceptedAt || call.ended) { | ||
| throw new Error('not-found'); | ||
| } | ||
|
|
||
| if (!call.uids.includes(uid)) { | ||
| throw new Error('not-found'); | ||
| } | ||
| if (!call.features.includes('conference-escalation')) { | ||
| throw new Error('feature-not-available'); | ||
| } | ||
|
|
||
| const user = await Users.findOneById(uid); | ||
| if (!user) { | ||
| throw new Error('internal-error'); | ||
| } | ||
|
|
||
| const url = await this.escalateVoiceCallToConference(user, call); | ||
|
|
||
| // If the peer has also escalated this call, then we can hangup as we join the conference | ||
| if (call.escalatedByPeerAt) { | ||
| void callServer.hangupEscalatedCall(call, { type: 'user', id: user._id }).catch((err) => { | ||
| logger.error({ msg: 'Unexpected error while hanging up a fully escalated voice call', err }); | ||
| }); | ||
| } | ||
|
|
||
| return url; | ||
| } catch (err) { | ||
| logger.debug({ msg: 'Unexpected error during escalation', err, uid, callId, call }); | ||
| throw err; | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing Permission and Room Access Validation in Voice Call Escalation
The REST API endpoint media-calls.escalate and its corresponding service method MediaCallService.escalateCall allow any authenticated user to escalate an active voice call to a video conference. However, the implementation lacks critical authorization checks:
- No Video Conference Permission Check: It does not verify if the escalating user has the necessary permissions to start or join video conferences (e.g.,
call-management). - No Room Access Validation: When escalating external calls, the target room defaults to the persistent chat external room (
Pexip_Integration_PersistentChat_ExternalRoom) configured in settings. The method never validates whether the useruidactually has access to this target room before creating the conference and returning a join URL.
This allows an attacker who is a participant in a voice call to bypass video conference permissions and potentially gain unauthorized access/join links to video conferences in restricted rooms.
Steps to Reproduce
- Establish an active voice call with the
conference-escalationfeature enabled. - Send a POST request to
/api/v1/media-calls.escalatewith thecallIdof the call as an authenticated user who does not have video conference permissions or access to the persistent chat external room. - Observe that the API successfully returns a conference join URL.
Fix with AI
A security vulnerability was found by Hacktron.
File: apps/meteor/server/services/media-call/service.ts
Lines: 484-522
Severity: high
Vulnerability: Missing Permission and Room Access Validation in Voice Call Escalation
Description:
The REST API endpoint `media-calls.escalate` and its corresponding service method `MediaCallService.escalateCall` allow any authenticated user to escalate an active voice call to a video conference. However, the implementation lacks critical authorization checks:
1. **No Video Conference Permission Check**: It does not verify if the escalating user has the necessary permissions to start or join video conferences (e.g., `call-management`).
2. **No Room Access Validation**: When escalating external calls, the target room defaults to the persistent chat external room (`Pexip_Integration_PersistentChat_ExternalRoom`) configured in settings. The method never validates whether the user `uid` actually has access to this target room before creating the conference and returning a join URL.
This allows an attacker who is a participant in a voice call to bypass video conference permissions and potentially gain unauthorized access/join links to video conferences in restricted rooms.
Proof of Concept:
**Steps to Reproduce**
1. Establish an active voice call with the `conference-escalation` feature enabled.
2. Send a POST request to `/api/v1/media-calls.escalate` with the `callId` of the call as an authenticated user who does not have video conference permissions or access to the persistent chat external room.
3. Observe that the API successfully returns a conference join URL.
Affected Code:
public async escalateCall(uid: IUser['_id'], params: { callId: string }): Promise<string> {
const { callId } = params;
logger.debug({ msg: 'Escalating Voice Call', method: 'MediaCallService.escalateCall', uid, callId });
const call = await MediaCalls.findOneById(callId);
try {
if (!call?.acceptedAt || call.ended) {
throw new Error('not-found');
}
if (!call.uids.includes(uid)) {
throw new Error('not-found');
}
if (!call.features.includes('conference-escalation')) {
throw new Error('feature-not-available');
}
const user = await Users.findOneById(uid);
if (!user) {
throw new Error('internal-error');
}
const url = await this.escalateVoiceCallToConference(user, call);
// If the peer has also escalated this call, then we can hangup as we join the conference
if (call.escalatedByPeerAt) {
void callServer.hangupEscalatedCall(call, { type: 'user', id: user._id }).catch((err) => {
logger.error({ msg: 'Unexpected error while hanging up a fully escalated voice call', err });
});
}
return url;
} catch (err) {
logger.debug({ msg: 'Unexpected error during escalation', err, uid, callId, call });
throw err;
}
}
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), !accepted_risk <reason>, or !fixed (resolved). Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
Co-authored-by: Aleksander Nicacio da Silva <aleksander.silva@rocket.chat> Co-authored-by: gabriellsh <henriques.gabriell@gmail.com> Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
Co-authored-by: Aleksander Nicacio da Silva <aleksander.silva@rocket.chat> Co-authored-by: gabriellsh <henriques.gabriell@gmail.com> Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
Co-authored-by: Aleksander Nicacio da Silva <aleksander.silva@rocket.chat> Co-authored-by: gabriellsh <henriques.gabriell@gmail.com> Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
Co-authored-by: Aleksander Nicacio da Silva <aleksander.silva@rocket.chat> Co-authored-by: gabriellsh <henriques.gabriell@gmail.com> Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
Co-authored-by: Aleksander Nicacio da Silva <aleksander.silva@rocket.chat> Co-authored-by: gabriellsh <henriques.gabriell@gmail.com> Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
Proposed changes (including videos or screenshots)
DMV-17
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit