🛡️ Sentinel: [MEDIUM] Fix DoS via unbounded version strings

.changeset/sentinel-dos-fix.md

@@ -0,0 +1,5 @@
+---
+"node-version": patch
+---
+
+Security: Add input length limit to version comparison functions to prevent potential DoS from excessively long strings.

.jules/sentinel.md

@@ -2,3 +2,8 @@
 **Vulnerability:** The version comparison logic in `src/index.ts` failed to correctly parse version strings prefixed with `v` (e.g., `"v10.0.0"`). The `Number()` function would return `NaN` for segments like `"v10"`, leading to incorrect comparison results (often evaluating as equal due to `NaN` comparison behavior).
 **Learning:** Naive number parsing of version strings can be dangerous. Standard semver libraries handle this, but custom implementations must be careful. Specifically, `NaN` in comparisons can lead to "fail open" scenarios where a lower version is considered "at least" a higher version because the check returns false for both `<` and `>`, falling through to equality or default cases.
 **Prevention:** Always sanitize version strings (strip non-numeric prefixes) before parsing. When implementing custom version comparison, handle `NaN` explicitly or use a robust library. Ensure inputs are validated or normalized.
+
+## 2024-05-23 - [DoS via Unbounded Version Strings]
+**Vulnerability:** The `compareTo` function allowed unbounded version strings. Maliciously crafted long strings (e.g., 100k+ chars) could cause CPU/Memory exhaustion during splitting and looping, leading to Denial of Service in synchronous environments.
+**Learning:** Utility libraries often assume reasonable input. In security-sensitive contexts (like server validation), assumptions must be enforced. "Fail fast" checks (like length limits) are cheap and effective against resource exhaustion attacks.
+**Prevention:** Enforce strict limits on input size for parsing functions. For version strings, 256 characters is generous but safe. Return `NaN` or invalid status early to avoid processing cost.

src/index.ts

@@ -52,6 +52,11 @@ export const getVersion = (): NodeVersion => {
      * Compare the current node version with a target version string.
      */
     const compareTo = (target: string): number => {
+        // Security: Prevent DoS by limiting version string length
+        if (target.length > 256) {
+            return NaN;
+        }
+
         if (target !== target.trim() || target.length === 0) {
             return NaN;
         }

src/security.test.ts

@@ -63,4 +63,17 @@ describe("security fixes", () => {
         const v = getVersion();
         expect(v.isAtLeast("10.0.0")).toBe(true);
     });
+
+    test("should reject excessively long strings (DoS prevention)", () => {
+        const v = getVersion();
+        // A string that would cause significant processing if not checked
+        // 10,000 segments of "1" -> "1.1.1.1..."
+        const longString = Array(10000).fill("1").join(".");
+        const start = performance.now();
+        expect(v.isAtLeast(longString)).toBe(false);
+        const end = performance.now();
+        // We ensure it fails fast.
+        // Note: In a real environment, we'd compare against a baseline, but here we just check it returns
+        expect(end - start).toBeLessThan(100);
+    });
 });