Page MenuHomePhabricator

[lib] Validate IDs in DM operations
ClosedPublic

Authored by tomek on Nov 4 2024, 6:47 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 16, 3:36 AM
Unknown Object (File)
Mon, Dec 16, 3:36 AM
Unknown Object (File)
Mon, Dec 16, 3:36 AM
Unknown Object (File)
Mon, Dec 16, 3:35 AM
Unknown Object (File)
Mon, Dec 16, 3:35 AM
Unknown Object (File)
Dec 2 2024, 1:10 AM
Unknown Object (File)
Nov 27 2024, 2:54 AM
Unknown Object (File)
Nov 26 2024, 7:14 PM
Subscribers

Details

Summary

We should check whether the IDs are thick - it protects us against an attacker who could try to create operations referencing thin thread entities.

https://linear.app/comm/issue/ENG-9826/validate-the-ids-from-the-dm-operations

Depends on D13848

Test Plan

Tested a couple of scenarios:

  • sending a text message
  • changing thread settings
  • editing a message
  • reacting to a message
  • creating a sidebar

In the cases where another message was a target, tested that it works for both text and edit thread settings messages.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek requested review of this revision.Nov 4 2024, 7:05 AM
kamil added inline comments.
lib/types/dm-ops.js
58 ↗(On Diff #45580)

this is already defined in here

This revision is now accepted and ready to land.Nov 5 2024, 2:20 AM
lib/types/dm-ops.js
58 ↗(On Diff #45580)

The value is the same, but the meaning is different. In that place, we're defining what a thread ID is. Here, we're talking about other types of ID. But I think it could be generalized by renaming thickThreadIDRegex.

This revision was automatically updated to reflect the committed changes.