Skip to content

[CRITICAL] Missing input validation in trading bot IPC handler #35

@FarahR01

Description

@FarahR01

Hello,

I really admire your project and the creativity behind it, and I wanted to start contributing to open-source projects. I came across your repository, forked it, and carefully went through the codebase.

The project is very well done and well-architected. However, I noticed an issue that I had previously encountered in one of my academic projects, so I thought it would be valuable to share it here.

I decided to open an issue to discuss it. If this causes any inconvenience or disruption, please let me know and I sincerely apologize in advance.

Kind regards,
Farah Rihane
Fresh Graduate Software Engineer

Problem Noticed !

The start-trading-bot IPC handler in main.js does not validate trading configuration before executing trades.
This allows a compromised renderer (XSS) to start a bot with:

  • Unlimited trade amounts (e.g., 999999999 units)
  • Invalid intervals that bypass risk manager
  • No validation against position size limits

Current Code (Line 728-732 in main.js):

if (!config || !config.apiConfig || !config.symbol || !config.strategy) {
  event.reply('trading-error', 'Invalid configuration provided');
  return;
}
// ❌ Only checks if fields exist, doesn't validate values

Attack Example:

// Attacker sends:
ipcRenderer.send('start-trading-bot', {
  amount: 999999999,      // No upper bound check
  interval: '999y',       // No interval validation
  strategy: 'invalid',    // No strategy validation
  apiConfig: { ... }
});
// ❌ Bot starts anyway and executes uncontrolled trades

Solution:

I Added a TradingBotValidator class to validate: electron/validators/tradingBotValidator.js
and updated main.js by adding 1 import line and the validation call
✓ Amount is positive and ≤ risk manager limit
✓ Interval is one of: 1m, 5m, 15m, 30m, 1h, 4h, 1d
✓ Strategy is one of: simpleMovingAverage, relativeStrengthIndex, bollingerBands
✓ API credentials have valid format

Impact:

  • User could lose entire balance in minutes
  • Risk manager limits are completely bypassed
  • All users affected if renderer is compromised

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    Status

    In Progress

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions