diff --git a/.changeset/sentinel-dos-fix.md b/.changeset/sentinel-dos-fix.md new file mode 100644 index 00000000..a03b914d --- /dev/null +++ b/.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. diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 356c7f03..06785ead 100644 --- a/.jules/sentinel.md +++ b/.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. diff --git a/src/index.ts b/src/index.ts index 625ca808..9ee788a1 100644 --- a/src/index.ts +++ b/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; } diff --git a/src/security.test.ts b/src/security.test.ts index b5942fd7..b8504434 100644 --- a/src/security.test.ts +++ b/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); + }); });