- Add input-validation.ts with regex, path, and URL validation utilities - Validate regex patterns before RegExp creation to prevent ReDoS - Block dangerous nested quantifiers (a+)+, (a*)+, etc. - Prevent path traversal with directory escape detection - Block localhost, private IPs, and non-http/https protocols for SSRF - Add SecurityOptions for configurable validation (allowPrivateIPs, etc.) - Include 33 security tests (unit + integration) Fixes #362 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
185 lines
5.8 KiB
Markdown
185 lines
5.8 KiB
Markdown
# Security Hardening Implementation
|
|
|
|
**Date**: 2026-01-12
|
|
**OpenProject**: [#362](https://pm.hyperlocalplatform.com/work_packages/362)
|
|
**Branch**: `feature/362-security-hardening`
|
|
**Status**: Complete
|
|
|
|
## Related Documents
|
|
- Task: `docs/tasks/security-hardening-task.md`
|
|
|
|
## Summary
|
|
|
|
Implemented security hardening to address three pre-existing vulnerabilities identified in the security audit:
|
|
|
|
1. **ReDoS (Regular Expression Denial of Service)** - Malicious regex patterns could cause exponential backtracking
|
|
2. **Path Traversal** - Malicious file paths could escape intended directories
|
|
3. **SSRF (Server-Side Request Forgery)** - Malicious URLs could access internal resources
|
|
|
|
## Files Modified
|
|
|
|
### New Files
|
|
| File | Purpose |
|
|
|------|---------|
|
|
| `src/lib/input-validation.ts` | Centralized input validation utilities |
|
|
| `scripts/self-testing/security-test.ts` | 25 security unit tests |
|
|
| `scripts/self-testing/mcp-integration-test.ts` | 8 MCP integration tests |
|
|
|
|
### Modified Files
|
|
| File | Changes |
|
|
|------|---------|
|
|
| `src/tools/query.ts` | Added regex validation before `new RegExp()`, import `createSafeRegex` |
|
|
| `src/lib/parser.ts` | Added `validateSpecPath()` call, security options in `ParseOptions` |
|
|
|
|
## Implementation Details
|
|
|
|
### ReDoS Protection
|
|
- Validates regex patterns before creating `RegExp` objects
|
|
- Blocks patterns with:
|
|
- Nested quantifiers: `(a+)+`, `(a*)+`, etc.
|
|
- Excessive length (>500 chars)
|
|
- Deep nesting (>10 levels)
|
|
- Lookahead/lookbehind patterns
|
|
|
|
```typescript
|
|
// In query.ts - validation before use
|
|
if (args.pathPattern) {
|
|
const regexValidation = validateRegexPattern(args.pathPattern);
|
|
if (!regexValidation.valid) {
|
|
return errorResponse(regexValidation.error, 'validating path pattern');
|
|
}
|
|
}
|
|
```
|
|
|
|
### Path Traversal Prevention
|
|
- Validates file paths stay within allowed base directories
|
|
- Detects traversal patterns: `../`, URL-encoded (`%2e%2e`), double-encoded
|
|
- Uses cross-platform path separator (`path.sep`)
|
|
|
|
```typescript
|
|
// Uses resolve() and startsWith() check
|
|
const resolvedPath = resolve(normalize(filePath));
|
|
const isWithinAllowed = allowedBaseDirs.some(baseDir => {
|
|
const resolvedBase = resolve(baseDir);
|
|
return resolvedPath.startsWith(resolvedBase + sep) || resolvedPath === resolvedBase;
|
|
});
|
|
```
|
|
|
|
### SSRF Protection
|
|
- Validates URL protocol (http/https only)
|
|
- Blocks localhost and loopback IPs
|
|
- Blocks private IP ranges (10.x, 172.16-31.x, 192.168.x)
|
|
- Blocks link-local addresses (169.254.x)
|
|
- Configurable via `allowPrivateIPs` option
|
|
|
|
```typescript
|
|
// Blocked hostnames and IP patterns
|
|
const BLOCKED_HOSTNAMES = ['localhost', '127.0.0.1', '::1', '0.0.0.0'];
|
|
const PRIVATE_IP_PATTERNS = [
|
|
/^127\./, /^10\./, /^172\.(1[6-9]|2[0-9]|3[0-1])\./, /^192\.168\./
|
|
];
|
|
```
|
|
|
|
## Security Options
|
|
|
|
New `SecurityOptions` interface for configurable security:
|
|
|
|
```typescript
|
|
interface SecurityOptions {
|
|
allowPrivateIPs?: boolean; // Allow internal IPs (default: false)
|
|
allowedBaseDirs?: string[]; // Allowed file directories
|
|
skipUrlValidation?: boolean; // Skip URL validation for trusted sources
|
|
}
|
|
|
|
// Usage
|
|
await parseSpec('spec.yaml', {
|
|
security: { allowPrivateIPs: true }
|
|
});
|
|
```
|
|
|
|
## Test Results
|
|
|
|
### Security Unit Tests (25 tests)
|
|
```
|
|
✅ Rejects nested quantifier pattern (a+)+
|
|
✅ Rejects nested quantifier pattern (a*)+
|
|
✅ Rejects nested quantifier pattern ([a-zA-Z]+)*
|
|
✅ Rejects overly long regex patterns
|
|
✅ Allows safe regex patterns
|
|
✅ createSafeRegex returns null for dangerous patterns
|
|
✅ createSafeRegex returns RegExp for safe patterns
|
|
✅ Rejects path with ../
|
|
✅ Rejects path with encoded traversal %2e%2e
|
|
✅ Rejects path with double encoded traversal
|
|
✅ Allows paths within current directory
|
|
✅ Allows absolute paths within cwd
|
|
✅ Rejects localhost URLs
|
|
✅ Rejects 127.0.0.1 URLs
|
|
✅ Rejects private IP 10.x.x.x
|
|
✅ Rejects private IP 172.16.x.x
|
|
✅ Rejects private IP 192.168.x.x
|
|
✅ Rejects file:// protocol
|
|
✅ Rejects ftp:// protocol
|
|
✅ Allows public HTTPS URLs
|
|
✅ Allows public HTTP URLs
|
|
✅ Allows private IPs when allowPrivateIPs is true
|
|
✅ validateSpecPath correctly identifies URLs
|
|
✅ validateSpecPath rejects dangerous URLs
|
|
✅ validateSpecPath rejects path traversal
|
|
```
|
|
|
|
### MCP Integration Tests (8 tests)
|
|
```
|
|
✅ query-endpoints rejects ReDoS pattern
|
|
✅ query-endpoints accepts safe regex
|
|
✅ parseSpec rejects path traversal
|
|
✅ parseSpec rejects localhost URL
|
|
✅ parseSpec rejects private IP
|
|
✅ parseSpec rejects file:// protocol
|
|
✅ parseSpec accepts valid local file
|
|
✅ parseSpec allows private IP with allowPrivateIPs option
|
|
```
|
|
|
|
## Known Limitations
|
|
|
|
Documented in code comments:
|
|
|
|
1. **DNS Rebinding**: Hostname validation checks the hostname string, not resolved IP. For full SSRF protection against DNS rebinding, additional measures would be needed.
|
|
|
|
2. **HTTP Redirects**: The swagger-parser library follows HTTP redirects. A malicious redirect could bypass URL validation.
|
|
|
|
For MCP server use cases (local CLI tool), these are acceptable limitations with reduced attack surface.
|
|
|
|
## Security Audit Summary
|
|
|
|
| Severity | Count | Status |
|
|
|----------|-------|--------|
|
|
| Critical | 0 | - |
|
|
| High | 2 | Documented limitations (DNS rebinding, redirects) |
|
|
| Medium | 3 | Low priority for CLI tool context |
|
|
| Low | 4 | Backlog items |
|
|
|
|
## Rollback Instructions
|
|
|
|
To rollback these changes:
|
|
|
|
1. Revert the input-validation.ts file:
|
|
```bash
|
|
git checkout HEAD~1 -- src/lib/input-validation.ts
|
|
```
|
|
|
|
2. Remove validation imports and calls from parser.ts and query.ts:
|
|
```bash
|
|
git checkout HEAD~1 -- src/lib/parser.ts src/tools/query.ts
|
|
```
|
|
|
|
3. Rebuild:
|
|
```bash
|
|
npm run build
|
|
```
|
|
|
|
## References
|
|
|
|
- OWASP ReDoS: https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
|
|
- OWASP Path Traversal: https://owasp.org/www-community/attacks/Path_Traversal
|
|
- OWASP SSRF: https://owasp.org/www-community/attacks/Server_Side_Request_Forgery
|