← Back to PRs

#17380: fix(imessage): reject non-numeric chat_id values to prevent silent misrouting

by aldoeliacim open 2026-02-15 18:02 View on GitHub →
channel: imessage stale size: XS trusted-contributor
## Problem `parseChatTargetPrefixesOrThrow` uses `Number.parseInt(value, 10)` to parse `chat_id:` targets but only checks `Number.isFinite()` on the result. Since `parseInt` parses leading digits and ignores trailing characters, hex identifiers like `2ecba0e20f3a4299b0efb228a5990731` silently parse to `2`, routing messages to the wrong chat with no error. ## Fix Add a strict `/^\d+$/` regex check before `parseInt` so non-numeric values throw a clear error suggesting `chat_identifier:` instead. ## Tests Added two test cases: - Hex identifier rejection (`chat_id:2ecba0e...`) - Mixed alphanumeric rejection (`chat_id:12abc`) All 12 tests in `targets.test.ts` pass. Fixes #17362 <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes a silent misrouting bug in `parseChatTargetPrefixesOrThrow` where `Number.parseInt("2ecba0e20f3a4299b0efb228a5990731", 10)` would return `2` (parsing only leading digits), causing messages to be routed to the wrong chat. The fix adds a strict `/^\d+$/` regex validation before calling `parseInt`, with a helpful error message suggesting `chat_identifier:` for non-numeric values. - The fix in `parseChatTargetPrefixesOrThrow` is correct and well-tested - Two new test cases cover hex identifier and mixed alphanumeric rejection - **Issue found**: The sibling function `parseChatAllowTargetPrefixes` in the same file (lines 105-113) has the identical `parseInt` + `isFinite` vulnerability and was not updated — a hex `chat_id` in an `allowFrom` entry would still silently match the wrong chat <h3>Confidence Score: 3/5</h3> - The targeted fix is correct, but the same bug remains in the allowlist parsing path which could cause incorrect access control behavior. - The fix for `parseChatTargetPrefixesOrThrow` is sound and well-tested. However, `parseChatAllowTargetPrefixes` in the same file has the identical vulnerability and was not addressed, meaning the parseInt leading-digits problem still exists in the allowlist code path. This is a partial fix. - `src/imessage/target-parsing-helpers.ts` — the `parseChatAllowTargetPrefixes` function (lines 105-113) needs the same strict numeric validation applied. <sub>Last reviewed commit: 6ede6e3</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs