Skip to content

Fix JSON injection crash in close reason handling#582

Merged
Sayrix merged 2 commits intoSayrix:mainfrom
Sim-hu:fix/json-injection-close-reason
Mar 21, 2026
Merged

Fix JSON injection crash in close reason handling#582
Sayrix merged 2 commits intoSayrix:mainfrom
Sim-hu:fix/json-injection-close-reason

Conversation

@Sim-hu
Copy link
Copy Markdown
Contributor

@Sim-hu Sim-hu commented Mar 20, 2026

Summary

  • Fix JSON injection in close.ts: The ticketClosed embed is built by serializing to JSON, doing string replacements, then parsing back. User-provided closereason text containing ", \, or other JSON-special characters would break the JSON structure and cause JSON.parse to throw. Fixed by escaping all replacement values with JSON.stringify(...).slice(1, -1) before insertion, which preserves JSON-special character escaping while removing the surrounding quotes.
  • Fix forEach(async ...) anti-pattern: Replaced invited.forEach(async ...) in close.ts with a for...of loop so permission overwrites are properly awaited. Also replaced reasons.forEach(async ...) in createTicket.ts with for...of (the callback there wasn't actually async-dependent, but the pattern is misleading and could hide errors).

Test plan

  • Set a ticket close reason containing ", \, and other JSON-special characters (e.g. He said "hello" and typed C:\path\to\file) — verify the bot does not crash and the embed displays correctly
  • Set a close reason with newlines — verify they are handled as before
  • Set a close reason with normal text — verify no regression
  • Close a ticket with multiple invited users — verify all permission overwrites are applied sequentially without errors

@Sim-hu Sim-hu requested review from Sayrix and zhiyan114 as code owners March 20, 2026 14:20
@Sayrix
Copy link
Copy Markdown
Owner

Sayrix commented Mar 20, 2026

Thank you for your contribution, do you also have a proof of concept of the bug occurring? Or is this a theoretical fix?

@Sim-hu
Copy link
Copy Markdown
Contributor Author

Sim-hu commented Mar 21, 2026

Yeah, it's reproducible. If a user sets a close reason containing a double quote or backslash (e.g. test "reason" here or test\nreason), the resulting string gets embedded directly into the JSON template without escaping. JSON.parse() then throws a SyntaxError and the bot crashes.

Steps to reproduce:

  1. Open a ticket
  2. Close it with a reason containing " — e.g. He said "no"
  3. The close embed fails to send and throws an unhandled JSON parse error

The fix uses JSON.stringify().slice(1, -1) to properly escape special characters before embedding them into the JSON string.

@Sayrix Sayrix merged commit 4f7491e into Sayrix:main Mar 21, 2026
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.

2 participants