Skip to content
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

Jump to bottom

[BUG] It's possible to break modmail's internal thread cache and simultanously open many threads #3022

Closed
bast0006 opened this issue May 8, 2021 · 16 comments · Fixed by #3028
Closed

[BUG] It's possible to break modmail's internal thread cache and simultanously open many threads #3022

bast0006 opened this issue May 8, 2021 · 16 comments · Fixed by #3028
Labels
bug This is a confirmed bug staged Staged for next version

Comments

@bast0006
Copy link

bast0006 commented May 8, 2021 •
edited
Loading

Describe the bug
If confirm_thread_creation is used, a user can break the thread creation flow and somehow cause modmail's DM response to freeze. This then results in modmail sending as many confirm_thread_creation prompts as messages were sent during the freeze, and if the user reacts to each one, they can open one thread per message.

Bot Info
Bot version (check with @modmail about): 3.9.4
Host method (Heroku, self-host, etc): self-hosted

To Reproduce
Steps to reproduce the behavior:

  1. Who can reproduce (ex. anyone, owners)? Anyone
  2. Where can it be reproduced (ex. in thread channels, recipient DM's)? DM's
  3. Done what to cause the error?
    .1. DM modmail, wait for the confirm_thread_creation message to come up.
    .2. Block modmail
    .3. React to create the thread and close the thread before modmail can respond
    .4. Unblock modmail, and while it's frozen (~10-15s depending), send a few messages. The effect is worse if you wait a short time between each one
    .5. Observe many confirm_thread_creation messages that all work.
  4. Any recently made changes to your bot? Some print statements, and requests hitting a channel category instead of the guild's text channels.

Error Logs
https://paste.pythondiscord.com/luyevadeyu.txt

Screenshots
https://cdn.discordapp.com/attachments/620607373828030464/840607305586049034/Screen_Shot_test1.png
https://cdn.discordapp.com/attachments/620607373828030464/840607475187449866/Screen_Shot_test2.png
https://cdn.discordapp.com/attachments/620607373828030464/840607476554792990/Screen_Shot_test3.png

Additional context
Add any other context about the problem here.
@bast0006 bast0006 added the maybe: bug An unconfirmed bug label May 8, 2021
@bast0006 bast0006 changed the title [BUG] It's possible to break modmail's internal thread cache and simultanously open many threads and lock up the bot at the same time [BUG] It's possible to break modmail's internal thread cache and simultanously open many threads May 8, 2021
@Jerrie-Aries
Copy link
Contributor

.3. React to create the thread and close the thread before modmail can respond

What do you mean by close the thread before modmail can respond?
Do you mean close the thread from thread channel (using command), or deny the thread creation (from DM)?

@bast0006
Copy link
Author

bast0006 commented May 9, 2021

Click both the check and X mark reactions before modmail is able to remove them

@Jerrie-Aries
Copy link
Contributor

Thanks for the explanation.
I tried/tested it, and it seemed like I couldn't reproduce it.

@bast0006
Copy link
Author

If you have an instance available for testing I'd love to reproduce it for you?

@Jerrie-Aries
Copy link
Contributor

Sure, can you DM me on discord? Jerrie#7234.

@StephenDaDev
Copy link
Member

I have been able to half reproduce this on the latest version, running on Heroku.

@StephenDaDev
Copy link
Member

Screen Shot 2021-05-10 at 10 51 46 AM

I have been able to fully reproduce this issue on the latest version, running on Heroku.

@StephenDaDev
Copy link
Member

2021-05-10 14:54:06 main[1331] - ERROR: Ignoring exception in on_message.
2021-05-10 14:54:06 main[1332] - ERROR: Unexpected exception:
Traceback (most recent call last):
File "/app/.heroku/python/lib/python3.9/site-packages/discord/client.py", line 333, in _run_event
await coro(*args, **kwargs)
File "/app/bot.py", line 1007, in on_message
await self.process_commands(message)
File "/app/bot.py", line 1014, in process_commands
return await self.process_dm_modmail(message)
File "/app/bot.py", line 814, in process_dm_modmail
thread = await self.threads.create(message.author, message=message)
File "/app/core/thread.py", line 1207, in create
await message.channel.send(
File "/app/.heroku/python/lib/python3.9/site-packages/discord/abc.py", line 904, in send
data = await state.http.send_message(channel.id, content, tts=tts, embed=embed,
File "/app/.heroku/python/lib/python3.9/site-packages/discord/http.py", line 241, in request
raise Forbidden(r, data)
discord.errors.Forbidden: 403 Forbidden (error code: 50007): Cannot send messages to this user
2021-05-10 14:54:17 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:31 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:31 core.thread[379] - ERROR: Thread already closed: 231595246213922828.
2021-05-10 14:54:31 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:31 core.thread[379] - ERROR: Thread already closed: 231595246213922828.
2021-05-10 14:54:35 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:35 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:35 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:35 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:35 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:35 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:35 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:36 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:36 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:37 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:37 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:37 core.thread[1074] - WARNING: Found existing thread for 231595246213922828 but the channel is invalid.
2021-05-10 14:54:38 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:40 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:43 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:44 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:45 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:47 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:49 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:50 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:51 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:52 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.
2021-05-10 14:54:53 main[1129] - WARNING: Failed to find linked message for reactions: Thread channel message not found.

Logs. Was able to reproduce a 2nd time.

@onerandomusername
Copy link

Can reproduce.

image

I've found a simpler reproduction step as well.

On step three, don't even both reacting to open a thread, just react to close the thread. It works better, and is more reliable to recreate the issue.

@onerandomusername
Copy link

I can reproduce successfully multiple times in a row

image

@Jerrie-Aries
Copy link
Contributor

I can reproduce successfully multiple times in a row

Did the bot create multiple channels for you as well?

@StephenDaDev
Copy link
Member

On step three, don't even both reacting to open a thread, just react to close the thread. It works better, and is more reliable to recreate the issue.

Just did this and it spammed me with "Cancelled."

@onerandomusername
Copy link

I can reproduce successfully multiple times in a row

Did the bot create multiple channels for you as well?

Yes

a lot of channels

@onerandomusername
Copy link

On step three, don't even both reacting to open a thread, just react to close the thread. It works better, and is more reliable to recreate the issue.

Just did this and it spammed me with "Cancelled."

You still need to react with the checkbox for all later messages. The first one during the block didn't the checkbox tho.

@StephenDaDev
Copy link
Member

Ahh! I confused the steps. Also have been able to reproduce on one version out of date, running on VPS, multiple times, same thing in the logs.

@Jerrie-Aries
Copy link
Contributor

Jerrie-Aries commented May 10, 2021 •
edited
Loading

Welp. I managed to reproduce this on my local machine.

Untitled

I had a patch ready for this issue a while back when I encountered similar issue using react to contact.

The issue is probably from here:
https://github.com/kyb3r/modmail/blob/3717df646644159c5a0e713aa83bfcdbf768dcdb/core/thread.py#L1120-L1128

Should also add not thread.cancelled as well in if statement to make sure there's no existing cancelled thread before returning None.
Returning None that makes the bot create a new thread.

Also need to handle the exception when sending "Cancelled" embed, this probably what causes the bot to hang without returning the Thread instance (which cancelled flag is set to True) to wherever it's called from.
https://github.com/kyb3r/modmail/blob/3717df646644159c5a0e713aa83bfcdbf768dcdb/core/thread.py#L1246-L1270

@fourjr fourjr mentioned this issue May 24, 2021
Merged
@fourjr fourjr linked a pull request May 24, 2021 that will close this issue
Fix breaking modmail internal thread cache #3028
Merged
@fourjr fourjr added bug This is a confirmed bug staged Staged for next version and removed maybe: bug An unconfirmed bug labels May 24, 2021
@fourjr fourjr closed this as completed in ef4b5b2 Sep 4, 2021
Agent-Fennec pushed a commit to Agent-Fennec/modmail that referenced this issue Mar 12, 2022
@pull @fourjr
@Jerrie-Aries @AyamDobhal @lorenzo132 @scragly @Taaku18 @6days9weeks @Coolguy3289 @infinitepower18 @redstonedesigner @StephenDaDev @RealCyGuy @nikkelma @Akarys42 @Qwerty-133 @popeeyy @TheDiscordHistorian
[pull] master from kyb3r:master (#70)
3a9e23f 
* Sending files in threads (non-images) now work. resolves modmail-dev#2926

* Deleting messages no longer shows a false error, resolves modmail-dev#2910

* Linting

* Fix `?perms get` command showing IDs instead of user or role mentions.

* Support discord.py v1.6 & LOTTIE stickers

* Release v3.8

* Linting & Changelog

* Fix bug when sending multiple images at once.

* Fix error when reacting on confirm thread creation message.

* Update changelog

* Autotriggers no longer sends attachments back. resolves modmail-dev#2932

* Fix `logs` command without argument in thread channel when the recipient is not cached.

* Fix error raised when recipient is not cached and reacts to reactions in DM channel.

* Fixed bug with update notifiations

* Retry with diff name if channel cant be created resolves modmail-dev#2934

* Retrieve user from Discord API if user has left the server, modmail-dev#2935 modmail-dev#2936

* IDs in `<member>` commands work now.

* update version in discord version error string

* Hotfix: Corrupted data no longer saved to thread cache

* Bump version

* 3.8.4 - Another fix attempt

* Improved cache saving methods

* Ability to disable mention on thread creation.

* Update thread move message in case `mention` was set to disable/None.

* Formatting...

* Apply suggestions from code review

Co-authored-by: Jia Rong Yee <28086837+fourjr@users.noreply.github.com>

* Fix typo in 'config_help.json' (modmail-dev#2957)

* Update README.md (modmail-dev#2955)

fix a sentence which said to join the old deleted plugin server

* Add msglink command to get DM message URLs.

* Add DM channel ID to genesis message footer.

* Amend typo and black formatting.

* Improve messages

* Changelog update

* Non-master/development branch deployments no longer cause erros to be raised.

* Update changelog

* Fix bug where autotriggers are in dm context, resolve modmail-dev#2961

* Use the ID attribute of DM Channel.

Originally used the channel instance by mistake, causing the `__str__` to be used instead of the intended ID.

* Confirm thread creation when user opens a thread using react to contact.

* Block new thread from react to contact when Modmail DM is disabled (new or all).

* Bot will not attempt to find for linked message when the recipient reacts to reactions on confirm thread creation message.

* Fix previous commit.

* Fix a few bad refs and typos

* Add local plugins

* Add local plugin dir

* Add local plugins

* Updated version, deps, and plugins code

* Forgot to finish sentence

* new plugin

A little plugin I was working on

* Potentially breaking: improved plugin events

* Fix bug where cancelled still attempts to start thread

* Changelog & Ghost errors are no longer raised when threads are created using non-organic methods.

* Add reaciton menu plugin

* better run command, 3.9.1

* add music plugin

added music plugin on taki's request

* Update registry.json

try to fix install issue

* Update bot.py

add hosting methods: screen and systemd

* Update models.py

add hosting methods: screen and systemd

* Force update dev? Weird history

* Add docker hostingmethod

* Update config_help.json

Change disabled to enabled, as it is enabled on a new bot creation

* Bump versions

* Update readme

* did i not commit this

* Update `mention` command.

* Add Coolguy (Prime Servers) to SPONSORS

* Small correction

* Add Coolguy (Prime Servers) to SPONSORS

* Update to HTTPS

* Update to HTTPS

* v3.9.3 add use_user_id_channel_name

* black

* Possibly fix emoji problem

* Cleaner code :D

* Bump python runtime version

Security update

* Update sponsors

* Add docker-compose configuration for self hosting

Adds a docker-compose configuration file for running both the bot and logviewer with a shared mongo database in a single container stack.

Exposes port 8000 and ensure the mongo database can not be accessed remotely.

* Change Max Channel per Category to 49, fix fallback save logic

* There is now a proper message when trying to contact a bot.

* Changelog - v3.9.4

* Fix typo

* add fourjr claim

* Add volume for persistent data storage to Mongo

* Update pyproject.toml

* Update README.md

* Update sponsors

* Fix typo in config help.

* Update 'bot.py' and 'clients.py':
- Fix return types, type hints, and unresolved references.

* Update database after resetting/purging all plugins.

* `plugin reset`, remove unloaded plugins from `self.loaded_plugins`

* turn mentions into set

* Hopefully resolves modmail-dev#3022.

* Improve contact and react to contact:
- Checks if user is blocked when `?contact` command or react to contact is used.

* Remove publish plugin

* Initial 3.9.5-dev1

* Update registry.json

removal of profanity filter, trying to download will result in a infinite download loop and will stop any plugin from loading/break the usage of removing/adding new plugins

* Group conversation ALPHA modmail-dev#143

* Remove debug

* Push ver

* Fix closes for group conversations

* Bump version

* Update registry.json

* Changelog

* Add solution to CERTIFICATE_VERIFY_FAILED

* New issue forms

* formatting

* Update SPONSORS.json

* fix issue template

* Update requirements system and removed poetry

* Sort pipfile

* Remove flake8 from req, use new rec line-width of 110, black format

* Remove flake8 from req, use new rec line-width of 110, black format

* Add dnspython (pymongo[srv]) as req

* Updates channel.move, bot.close, emoji changes

* Keep more log files

* Bump version

* Black format

* Updated sponsors

* Improved dockerfile

* Add restart and environment for bot

* Update sponsors

* Update modmail.py

fix typo { modmail-dev#3067 } { https://usagi.xn--6frz82g/6DEshJ }

* Formatting and final updates to deps

* formatting

* Fix SSL Error

* Changelog

* bump v

* Fix failing Docker build

* Fix UnicodeEncodeError on Windows. (modmail-dev#3043)

* Fix UnionEncodeError on Windows when logging unicode emojis or special characters.

* Formatting...

* Plugins: add Python Discord's

- **Case insensitive snippets**: Allow snippets to be ran even if with the wrong case. For example, `?Dm-RePort` will be recognized as `?dm-report`.
- **Close message:** Add a `?closemessage` command that will close the thread after 15 minutes with a default message.
- **MDLink**: Generate a ready to paste link to the thread logs.
- **Reply cooldown**: Forbid you from sending the same message twice in ten seconds.
- **Tagging**: Add a `?tag` command capable of adding a `$message｜` header to the channel name.

* Let snippets be invoked case-insensitively.

* Update changelogs

* Update changelog

* Improved way to do case insensitive snippets

* Update changelog and merge issues

* Formatting

* Fix contact with category and silent, modmail-dev#3076

* Resolve close_on_leave_reason not properly working

* Invalid arguments are now properly catched and a proper error message is sent (BadUnionArg)

* Removeuser (resolve modmail-dev#3065), silent adding/removing (modmail-dev#3054)

* Formatting

* add multiple users to a thread (resolve modmail-dev#3066), new group config options

* Formatting

* Fix formatter, allow add by roles (resolve modmail-dev#3053)

* Remove tagging and case insensitive plugins

* Changelog

* Use highest hoisted role as default tag, resolves modmail-dev#3014

* Reload thread cache only when it's the first on_ready trigger. Resolves modmail-dev#3037

* Resolve linked messages issues, improve group functionality (bugfixes), resolves modmail-dev#3041

* Persistent notes are now properly deleted,resolve  modmail-dev#3013

* Fix changelog formatting

* Fix changelog formatting

* Fix changelog formatting

* New thread related config, resolves modmail-dev#3072

* Resolve issues with deleting messages

* use_timestamp_channel_name config

* formatting

* add documentation and fix timestamp related bugs

* Initial commit

* Bug fixes and debug removal

* Remove debug

* Remove additional debug

* Update bot.py

* Edit error message

* move format_channel_name to Bot, resolve modmail-dev#2982

* Formatting

* bump ver

* Bug fixes

* Redo alias conversion

* Remove redundant category checks

* ?contact accepts multiple users, or role modmail-dev#3082

* Changelog and fix bot perm level

* Remove extra loop.close

* Push version to 3.10

* Quick bugfix on config help and debug hastebin

* Fix bug where snippet add did not check command name

* v3.10.1 - fix edit cmd

* Fix contact

* bump ver

* oops

* Add phish checker plugin

* Added sponsor

* Update SPONSORS.json

* Update bandit baseline:

* 3.10.3 contact fix

Co-authored-by: Jia Rong Yee <28086837+fourjr@users.noreply.github.com>
Co-authored-by: Jerrie-Aries <70805800+Jerrie-Aries@users.noreply.github.com>
Co-authored-by: Ayam Dobhal <dobhalayam@gmail.com>
Co-authored-by: lorenzo132 <50767078+lorenzo132@users.noreply.github.com>
Co-authored-by: scragly <29337040+scragly@users.noreply.github.com>
Co-authored-by: Taku <45324516+Taaku18@users.noreply.github.com>
Co-authored-by: ❥sora <60399731+6days9weeks@users.noreply.github.com>
Co-authored-by: Ralph <blackbirdralph@aol.com>
Co-authored-by: codeinteger6 <44692189+codeinteger6@users.noreply.github.com>
Co-authored-by: redstonedesigner <redstonedesigner1@gmail.com>
Co-authored-by: Stephen <48072084+StephenDaDev@users.noreply.github.com>
Co-authored-by: Cyrus Yip <cyruscmyip1@gmail.com>
Co-authored-by: Cyrus <54488650+RealCyGuy@users.noreply.github.com>
Co-authored-by: Matt Nikkel <matt.nikkel@hey.com>
Co-authored-by: Matteo Bertucci <matteobertucci2004@gmail.com>
Co-authored-by: Qwerty-133 <74311372+Qwerty-133@users.noreply.github.com>
Co-authored-by: popeeyy <29686338+popeeyy@users.noreply.github.com>
Co-authored-by: kato <78689486+TheDiscordHistorian@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug This is a confirmed bug staged Staged for next version
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix breaking modmail internal thread cache Jerrie-Aries/modmail
5 participants
@fourjr @StephenDaDev @bast0006 @Jerrie-Aries @onerandomusername