Vulnerabilities(ciritcal level=3/10)

[Anonymi69]

Security Review — TitanBot

This document summarizes two issues found during a manual review of the codebase, along with suggested fixes. Neither is remote code execution or a backdoor — the codebase was checked for those (eval/Function/vm usage, child_process calls, SQL injection, hardcoded owner IDs) and came back clean. These are narrower, real bugs worth patching.

---

1. /say allows mention escalation (src/commands/Moderation/say.js)

Severity: Medium

Issue:
The /say command only requires the Manage Messages permission. The message content is passed through sanitizeInput() (src/utils/validation.js), which strips control characters and truncates length — but does not strip @everyone, @here, or role mentions. The command also never sets allowedMentions when calling channel.send().

Impact:
Any moderator with Manage Messages — a permission commonly given to junior/trial mods without also giving Mention Everyone — can type /say message:@everyone <text> and have the bot ping the whole server, riding on the bot's own (usually broad) permissions. This bypasses Discord's own per-user mention permission and can be used for spam, griefing, or by a compromised low-trust mod account to cause maximum disruption.

Fix:
Restrict allowedMentions on the outgoing message so only explicitly-permitted mention types go through, e.g.:

const sentMessage = await channel.send({
  content: message,
  allowedMentions: { parse: [] }, // blocks @everyone/@here/role/user pings by default
});

If pinging specific users/roles should still be allowed for higher-permission staff, gate it behind an additional permission check (e.g. require MentionEveryone on the invoking member before allowing @everyone/@here/role mentions to pass through), rather than relying on Manage Messages alone.

---

2. Economy race condition allows currency duplication (src/services/economyService.js, src/utils/economy.js)

Severity: Medium

Issue:
Economy balance updates (transferMoney, and the underlying getEconomyData / setEconomyData helpers) follow a read-then-write pattern:

1. Read the user's current wallet balance.
2. Compute the new balance in application code.
3. Write the new balance back.

There is no database transaction, row lock (SELECT ... FOR UPDATE), or optimistic-concurrency check (e.g. version column / WHERE balance = $expected) anywhere in this path.

Impact:
If a user fires two or more economy actions (/pay, /gamble, /rob, /work, etc.) at nearly the same time — trivially done by spam-clicking, using multiple devices, or scripting rapid interactions — both requests can read the same starting balance before either write commits. Each write then applies its own delta on top of that stale balance, so the effects stack instead of applying sequentially. This can be used to duplicate currency (e.g. paying the same money to two people, or gambling and paying simultaneously) rather than spending it once.

Fix:
Wrap balance mutations in a single atomic database operation instead of separate read/compute/write steps. Two common approaches:

A. Atomic UPDATE (simplest, recommended for single-balance ops):

UPDATE economy
SET wallet = wallet - $1
WHERE guild_id = $2 AND user_id = $3 AND wallet >= $1
RETURNING wallet;

If no row is returned, the user didn't have enough funds — reject the action. This removes the read-then-write gap entirely.

B. DB transaction with row locking (needed for multi-row ops like transferMoney, which touches sender and receiver):

const client = await pool.connect();
try {
  await client.query('BEGIN');
  const { rows: [sender] } = await client.query(
    'SELECT wallet FROM economy WHERE guild_id=$1 AND user_id=$2 FOR UPDATE',
    [guildId, senderId]
  );
  if (sender.wallet < amount) throw new Error('Insufficient funds');
  await client.query(
    'UPDATE economy SET wallet = wallet - $1 WHERE guild_id=$2 AND user_id=$3',
    [amount, guildId, senderId]
  );
  await client.query(
    'UPDATE economy SET wallet = wallet + $1 WHERE guild_id=$2 AND user_id=$3',
    [amount, guildId, receiverId]
  );
  await client.query('COMMIT');
} catch (e) {
  await client.query('ROLLBACK');
  throw e;
} finally {
  client.release();
}

FOR UPDATE locks the sender's row for the duration of the transaction, so a second concurrent transfer from the same user has to wait until the first commits (and will then correctly see the reduced balance).

---

Not vulnerable (checked, ruled out)

For context, since this review was prompted by a general "is this safe" concern:

• No eval/Function/vm usage reachable with unsanitized input (the two Function() calls found are whitelist-restricted to digits and arithmetic operators only — no letters allowed, so no code injection).
• No child_process/exec/spawn reachable from Discord commands (only used in backup/restore scripts run manually on the host).
• No SQL injection — all queries use parameterized $1/$2 placeholders; the only string-interpolated values are fixed table names from config, never user input.
• No hardcoded/secret owner ID — bot owner permissions come only from an OWNER_IDS environment variable set by the host.
• Ban/kick/timeout commands correctly check both moderator-vs-target and bot-vs-target role hierarchy before acting.
• No unguarded JSON.parse on user-controlled input (no prototype-pollution surface).