Skip to content

fix(e2ee): stop sending msg when there's content #6818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegolmello merged 1 commit into from
Nov 27, 2025
Merged

fix(e2ee): stop sending msg when there's content #6818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 6 additions & 33 deletions app/lib/encryption/room.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import getSingleMessage from '../methods/getSingleMessage';
import type {
IAttachment,
IMessage,
IUpload,
TSendFileMessageFileInfo,
IServerAttachment,
TSubscriptionModel,
Expand Down Expand Up @@ -482,46 +481,20 @@ export default class EncryptionRoom {
try {
const content = await this.encryptText(EJSON.stringify({ msg: message.msg || '' }));

return {
const encryptedMessage = {
...message,
t: E2E_MESSAGE_TYPE,
e2e: E2E_STATUS.PENDING,
e2eMentions: getE2EEMentions(message.msg),
content
};
} catch (e) {
// Do nothing
console.error(e);
}
} as IMessage;

return message;
};

// Encrypt upload
encryptUpload = async (message: IUpload) => {
if (!this.ready) {
return message;
}
delete encryptedMessage.msg;

try {
let description = '';

if (message.description) {
const encryptedResult = await this.encryptText(EJSON.stringify({ msg: message.description }));
description =
encryptedResult.algorithm === 'rc.v1.aes-sha2'
? encryptedResult.ciphertext
: EJSON.stringify({ kid: encryptedResult.kid, iv: encryptedResult.iv, ciphertext: encryptedResult.ciphertext });
}

return {
...message,
t: E2E_MESSAGE_TYPE,
e2e: E2E_STATUS.PENDING,
description
};
} catch {
return encryptedMessage;
} catch (e) {
// Do nothing
console.error(e);
}

return message;
Comment on lines +495 to 500

@coderabbitai coderabbitai Bot Nov 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all calls to .encrypt() and examine their context

echo "=== Finding .encrypt() method calls ==="
rg -n -C4 '\.encrypt\s*\(' --type=ts | head -100

echo ""
echo "=== Finding Encryption class references ==="
rg -n -C3 'Encryption\.(encrypt|encryptMessage)' --type=ts | head -50

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 2738

🏁 Script executed:

#!/bin/bash
# Examine Encryption.encryptMessage() implementation

echo "=== Examining Encryption class encryptMessage method ==="
rg -n -A20 'encryptMessage\s*\(' app/lib/encryption/encryption.ts | head -80

echo ""
echo "=== Examining sendMessage.ts context around Encryption.encryptMessage calls ==="
sed -n '60,120p' app/lib/methods/sendMessage.ts

echo ""
echo "=== Checking if there's message validation after encryption ==="
rg -n -C5 'content' app/lib/methods/sendMessage.ts | head -60

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1717

🏁 Script executed:

#!/bin/bash
# Get full context of Encryption class

echo "=== Finding Encryption.encryptMessage full implementation ==="
rg -n 'encryptMessage' app/lib/encryption/encryption.ts -A30 | head -100

echo ""
echo "=== Finding sendMessageCall implementation ==="
rg -n -B3 -A15 'sendMessageCall\s*\(' app/lib/methods/sendMessage.ts | head -80

echo ""
echo "=== Full context of EncryptionRoom.encrypt() in room.ts ==="
sed -n '470,510p' app/lib/encryption/room.ts

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 3288

🏁 Script executed:

#!/bin/bash
# Check if there's any server-side validation or other client guards

echo "=== Search for any validation of encrypted content before sending ==="
rg -n 'chat\.sendMessage' app/lib --type=ts -B5 -A5

echo ""
echo "=== Check for any guards on message.content or message.msg ==="
rg -n -C3 'message\.content\|message\.msg' app/lib/methods/sendMessage.ts

echo ""
echo "=== Check what happens with undefined return from encryptMessage ==="
rg -n -C5 'Encryption\.encryptMessage' app/lib/methods/sendMessage.ts

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1629

🏁 Script executed:

#!/bin/bash
# Final check: Look at what properties the encrypted message should have

echo "=== Check IMessage type to understand expected structure ==="
fd -type f -name "*.ts" | xargs grep -l "interface IMessage\|type IMessage" | head -5

echo ""
echo "=== Check E2E_MESSAGE_TYPE to understand encrypted messages ==="
rg -n "E2E_MESSAGE_TYPE\|E2E_STATUS" app/lib/encryption --type=ts -B2 -A2 | head -40

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 391

Based on my verification, I have found critical security issues with the error handling in the encryption code. Let me provide the rewritten review comment:

Encryption failure silently sends plaintext messages without validation.

When encryption fails (lines 495-500), EncryptionRoom.encrypt() returns the original unencrypted message. This fallback is problematic in encrypted rooms because:

  1. Encryption.encryptMessage() can return unencrypted messages or undefined:

    • Line 542: Returns undefined if room has no session key
    • Line 551: Returns unencrypted message if catch block executes
    • Line 537: Returns unencrypted message for non-encrypted subscriptions (intentional but indistinguishable from failures)

  2. Calling code has no validation:

    • sendMessage() and resendMessage() (app/lib/methods/sendMessage.ts) don't validate the return value
    • They pass whatever is returned directly to sendMessageCall() which sends it to the API
    • No check for undefined, no check for plaintext msg field vs encrypted content field

  3. Inconsistent with editMessage():

    • editMessage() (app/lib/services/restApi.ts:967-969) explicitly checks: if (!result) throw new Error('Failed to encrypt message')
    • Message send paths lack this guard, creating different security guarantees

Required action: Add explicit null/undefined check in sendMessage() and resendMessage() before calling sendMessageCall(), and/or differentiate between intentional fallback (non-encrypted room) vs error cases. Users expect E2E encryption; silent plaintext fallback is a security risk.

🤖 Prompt for AI Agents 
In app/lib/encryption/room.ts around lines 495-500, EncryptionRoom.encrypt()
currently catches errors and returns the original plaintext message, which can
cause silent plaintext sends; change behavior so encryption failures do not
return the original plaintext (either return undefined/null or throw) and ensure
the catch logs error details but does not fall back to plaintext. Then update
callers app/lib/methods/sendMessage.ts (sendMessage and resendMessage) to
explicitly check the encrypt() result before calling sendMessageCall(): if
encrypt() returns undefined/null or an error is thrown, abort sending and
surface/throw a clear "Failed to encrypt message" error (or return a failure
result) rather than sending plaintext; alternatively, make encrypt() return a
distinct sentinel for non-encrypted rooms and have callers treat that
differently from an encryption failure.

Expand Down
2 changes: 1 addition & 1 deletion app/lib/methods/sendFileMessage/sendFileMessageV2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export async function sendFileMessageV2(
'Content-Type': 'application/json'
},
body: JSON.stringify({
msg: file.msg || undefined,
msg: (content ? '' : file.msg) || undefined,
tmid: tmid || undefined,
description: file.description || undefined,
t: content ? 'e2e' : undefined,
Expand Down
Loading