Skip to content

fix(economy): prevent code injection via currency symbol/name #163

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

Open
Niruhqs wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(economy): prevent code injection via currency symbol/name #163

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions src/commands/Economy/modules/economy_dashboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,27 @@ async function updateConfigFile(currencySymbol, currencyName) {
const configPath = path.join(__dirname, '../../../config/bot.js');
let configContent = await fs.readFile(configPath, 'utf-8');

// Serialize as proper JS string literals (escapes quotes/backslashes) instead of
// raw-interpolating user input into source code - see security review, issue #151 follow-up.
const symbolLiteral = JSON.stringify(currencySymbol);
const nameLiteral = JSON.stringify(currencyName);
const namePluralLiteral = JSON.stringify(`${currencyName}s`);

configContent = configContent.replace(
/symbol:\s*"[^"]*"/,
`symbol: "${currencySymbol}"`
`symbol: ${symbolLiteral}`
);

configContent = configContent.replace(
/name:\s*"[^"]*",\s*\/\/\s*Currency display name/,
`name: "${currencyName}", // Currency display name`
`name: ${nameLiteral}, // Currency display name`
);

configContent = configContent.replace(
/namePlural:\s*"[^"]*",\s*\/\/\s*Plural display name/,
`namePlural: "${currencyName}s", // Plural display name`
`namePlural: ${namePluralLiteral}, // Plural display name`
);

await fs.writeFile(configPath, configContent, 'utf-8');
logger.info('Config file updated successfully');
return true;
Expand All @@ -139,6 +145,15 @@ async function updateConfigFile(currencySymbol, currencyName) {
}
}

// Only letters, numbers, currency symbols, and common punctuation - excludes quote/backslash/backtick
// so this can never contain characters that would break out of a string literal, even if a future
// refactor bypasses the JSON.stringify() call above.
const SAFE_CURRENCY_CHARS = /^[\p{L}\p{N}\p{Sc}\s.,!?-]+$/u;

function isSafeCurrencyInput(value) {
return SAFE_CURRENCY_CHARS.test(value);
}

export default {
prefixOnly: false,
async execute(interaction, config, client) {
Expand Down Expand Up @@ -453,6 +468,11 @@ async function handleChangeCurrency(selectInteraction, rootInteraction, guild) {
return;
}

if (!isSafeCurrencyInput(newSymbol)) {
await replyUserError(submitted, { type: ErrorTypes.VALIDATION, message: 'Currency symbol contains an unsupported character. Quotes, backslashes, and backticks are not allowed.' });
return;
}

const success = await updateConfigFile(newSymbol, BotConfig.economy.currency.name);

if (!success) {
Expand Down Expand Up @@ -507,6 +527,11 @@ async function handleChangeName(selectInteraction, rootInteraction, guild) {
return;
}

if (!isSafeCurrencyInput(newName)) {
await replyUserError(submitted, { type: ErrorTypes.VALIDATION, message: 'Currency name contains an unsupported character. Quotes, backslashes, and backticks are not allowed.' });
return;
}

const success = await updateConfigFile(BotConfig.economy.currency.symbol, newName);

if (!success) {
Expand Down