From 3b35f82d80b72c93511d5063db1bf4479e6c932e Mon Sep 17 00:00:00 2001 From: Rene Arumetsa Date: Mon, 10 Aug 2026 18:49:25 +0300 Subject: [PATCH] Fix RequestView over-funding reentrancy (do_give double-transfer) A follow-up reentrancy sweep of the five remaining money-moving views found one more instance of the same bug class. The other four (quests, prestige, fish, shop) verified clean - their underlying do_* functions are idempotent. RequestView / FundModal (commands/economy_support_commands.py): - on_submit read self._view.remaining, then awaited do_give, then decremented remaining. Because discord.py dispatches each modal submit as its own task and do_give is a plain non-idempotent transfer, a funder could open two modals and submit both before the first resolved: both read the same pre-decrement remaining, both passed the range check, and both transferred `amount` - over-funding the request (remaining goes negative) and moving up to the funder's whole balance. - Fix: reserve the amount synchronously (decrement remaining BEFORE the do_give await, with no await in between - atomic under asyncio), and roll the reservation back if the transfer fails. The second concurrent submit now sees the reduced remaining and is rejected. Added a cheap _fund guard (remaining<=0 / is_finished) so a click on a funded request doesn't open a dead modal. Co-Authored-By: Claude Opus 4.8 (1M context) --- commands/economy_support_commands.py | 11 ++++++++++- strings.py | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/commands/economy_support_commands.py b/commands/economy_support_commands.py index 4d4c25d..5d11e56 100644 --- a/commands/economy_support_commands.py +++ b/commands/economy_support_commands.py @@ -38,15 +38,21 @@ def register_economy_support_commands( ) return + # Reserve the amount synchronously (no await between the range check + # and the decrement) so two concurrent modal submits can't both fund + # the same request. discord.py runs each submit as its own task and + # do_give is not idempotent, so without this reservation a funder + # could double-submit and transfer `amount` more than once. + self._view.remaining -= amount res = await economy.do_give(interaction.user.id, self._view.requester.id, amount) if not res["ok"]: + self._view.remaining += amount # roll back the reservation data = await economy.get_user(interaction.user.id) await interaction.response.send_message( S.ERR["broke"].format(bal=coin(data["balance"])), ephemeral=True ) return - self._view.remaining -= amount funded_line = S.REQUEST_UI["funded_line"].format( name=interaction.user.display_name, amount=coin(amount), @@ -84,6 +90,9 @@ def register_economy_support_commands( self.add_item(self.fund_btn) async def _fund(self, interaction: discord.Interaction): + if self.remaining <= 0 or self.is_finished(): + await interaction.response.send_message(S.ERR["request_closed"], ephemeral=True) + return if interaction.user.id == self.requester.id: await interaction.response.send_message(S.ERR["request_self_fund"], ephemeral=True) return diff --git a/strings.py b/strings.py index 5ef005e..797e450 100644 --- a/strings.py +++ b/strings.py @@ -674,6 +674,7 @@ ERR: dict[str, str] = { "request_self": "❌ Sa ei saa iseendalt anuda.", "request_bot": "❌ Botid on teatavasti kitsid.", "request_targeted": "❌ See taotlus on suunatud kasutajale **{name}**.", + "request_closed": "❌ See taotlus on juba rahastatud või aegunud.", "already_in_game": "❌ Sul on juba aktiivne mäng käimas!", "invalid_amount": "❌ Sisesta kehtiv summa või 'all'.", "fund_range": "❌ Sisesta summa vahemikus 1-{max}.",