Buggy client or attacker sends NaN as a coordinate ,Results in Carsh of application in evaluations

[aniket866]

Fixes #90

Stability: Missing Invalid Number Checks (NaN)
Issue: The dx/dy values are not checked for NaN (Not a Number) before math operations. Note: Math.min(NaN, 100) returns NaN.

Fix: Add strict checks: if (!Number.isFinite(msg.dx)) return;.

Why?: If a buggy client or attacker sends NaN as a coordinate, calculations involving currentPos.x + NaN result in NaN. Passing NaN coordinates to the OS automation library (nut.js) typically causes the Node.js process to crash with an unhandled exception.

Checklist

• My PR addresses a single issue, fixes a single bug or makes a single improvement.
• My code follows the project's code style and conventions
• If applicable, I have made corresponding changes or additions to the documentation
• If applicable, I have made corresponding changes or additions to tests
• My changes generate no new warnings or errors
• I have joined the Discord server and I will share a link to this PR with the project maintainers there
• I have read the Contribution Guidelines
• Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.

⚠️ AI Notice - Important!

We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.

@imxade Please merge this fix

Summary by CodeRabbit

• Bug Fixes
  • Enhanced input validation for mouse movement controls to prevent non-numeric and infinite values from triggering unintended movement actions, improving input handling robustness.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added strict numeric type validation using typeof checks and Number.isFinite() to the mouse movement input handler. This prevents NaN or infinite coordinate values from being processed, avoiding downstream crashes when invalid values reach the OS automation library.

Changes

Cohort / File(s)	Summary
Input Validation Enhancement src/server/InputHandler.ts	Replaced simple existence check for dx/dy with strict numeric validation (typeof and Number.isFinite) to reject NaN and infinite values before they are applied to mouse position calculations.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A rabbit hops through code so clean,
Where NaN once caused a scene,
Now numbers pass the strictest test,
No infinite hops—just the best!
Validation guards the mouse's way,
Safe coordinates save the day! 🥕

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main bug fix: preventing NaN coordinates from crashing the application, which is the primary change in the PR.
Linked Issues check	✅ Passed	The PR successfully implements the requirements from issue #90 by adding strict numeric validation using Number.isFinite() to reject NaN/non-finite dx/dy values before processing.
Out of Scope Changes check	✅ Passed	All changes are directly related to the stated objective: the modifications to InputHandler.ts add validation for numeric inputs as required by issue #90.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/server/InputHandler.ts (2)

50-73: ⚠️ Potential issue | 🟠 Major

Same NaN vulnerability exists in the scroll case.

typeof msg.dy === 'number' && msg.dy !== 0 does not reject NaN — typeof NaN === 'number' is true and NaN !== 0 is true. This means mouse.scrollDown(NaN) can still be called, which is the exact same class of crash the PR aims to fix for move.

The same applies to msg.dx on line 63.

Proposed fix

-                if (typeof msg.dy === 'number' && msg.dy !== 0) {
+                if (typeof msg.dy === 'number' && Number.isFinite(msg.dy) && msg.dy !== 0) {
                     if (msg.dy > 0) {
                         promises.push(mouse.scrollDown(msg.dy));
                     } else {
                         promises.push(mouse.scrollUp(-msg.dy));
                     }
                 }

-                if (typeof msg.dx === 'number' && msg.dx !== 0) {
+                if (typeof msg.dx === 'number' && Number.isFinite(msg.dx) && msg.dx !== 0) {
                     if (msg.dx > 0) {
                         promises.push(mouse.scrollRight(msg.dx));
                     } else {
                         promises.push(mouse.scrollLeft(-msg.dx));
                     }
                 }

---

76-93: ⚠️ Potential issue | 🟠 Major

Same NaN vulnerability exists in the zoom case.

msg.delta !== undefined && msg.delta !== 0 passes NaN through, which then propagates via Math.sign(NaN) → NaN all the way to mouse.scrollDown(NaN).

Proposed fix

-                if (msg.delta !== undefined && msg.delta !== 0) {
+                if (typeof msg.delta === 'number' && Number.isFinite(msg.delta) && msg.delta !== 0) {