-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix(vulns): fix various vulnerabilities and enhanced code security #1611
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR significantly improves the security posture by addressing SSRF (Server-Side Request Forgery) and path traversal vulnerabilities across the codebase.
Key Changes:
- Added comprehensive input validation library (
apps/sim/lib/security/input-validation.ts) with functions to validate UUIDs, path segments, URLs, hostnames, and service-specific IDs - Applied validation to 30+ API routes including Jira, Confluence, Wealthbox, OneDrive, SharePoint, and Discord integrations
- Updated proxy routes to use new SSRF protections (blocks private IPs, localhost, and dangerous ports)
- Fixed string escaping vulnerability in condition evaluation logic
- Added HTML sanitization for Confluence content processing
- Comprehensive test coverage with 590+ test cases for validation functions
Security Improvements:
- Prevents path traversal attacks with strict path segment validation
- Blocks SSRF attempts by validating URLs against private IP ranges and localhost
- Validates Microsoft Graph IDs, Jira cloud IDs, and issue keys to prevent injection
- Blocks null bytes, URL encoding attacks, and directory separators in user input
Issues Found:
- ReDoS vulnerability: Three HTML sanitization functions use unbounded regex loops that could cause denial of service with malicious input (see inline comments)
Confidence Score: 3/5
- This PR improves security significantly but introduces ReDoS vulnerabilities that must be fixed before merge
- The PR adds comprehensive input validation that prevents critical SSRF and path traversal vulnerabilities. However, three HTML sanitization functions contain unbounded regex loops that create ReDoS (Regular Expression Denial of Service) vulnerabilities. These could allow attackers to cause performance degradation or service unavailability by submitting malicious HTML strings. The validation library itself is well-designed with good test coverage.
- Critical attention needed:
apps/sim/tools/confluence/utils.tsandapps/sim/lib/copilot/tools/server/other/make-api-request.ts- fix ReDoS vulnerabilities before merging
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/lib/security/input-validation.ts | 5/5 | New comprehensive input validation library added with functions to prevent path traversal, SSRF, and injection attacks |
| apps/sim/app/api/proxy/route.ts | 5/5 | Updated to use new URL validation library for proxy requests to prevent SSRF |
| apps/sim/tools/confluence/utils.ts | 3/5 | Added HTML sanitization functions with potential ReDoS vulnerability in regex loops |
| apps/sim/executor/resolver/resolver.ts | 4/5 | Fixed string escaping in condition evaluation to properly handle backslashes and prevent injection |
| apps/sim/lib/copilot/tools/server/other/make-api-request.ts | 5/5 | HTML stripping logic implemented, no changes made to validation |
Sequence Diagram
sequenceDiagram
participant Client
participant API Route
participant Validation
participant External API
Client->>API Route: Request with user input
API Route->>Validation: validatePathSegment(itemId)
alt Invalid Input
Validation-->>API Route: {isValid: false, error}
API Route-->>Client: 400 Bad Request
else Valid Input
Validation-->>API Route: {isValid: true, sanitized}
API Route->>Validation: validateProxyUrl(url)
alt SSRF Attempt
Validation-->>API Route: {isValid: false, error}
API Route-->>Client: 403 Forbidden
else Safe URL
Validation-->>API Route: {isValid: true}
API Route->>External API: Authenticated request
External API-->>API Route: Response
API Route-->>Client: Success response
end
end
Additional Comments (1)
-
apps/sim/lib/copilot/tools/server/other/make-api-request.ts, line 52-69 (link)logic: Same ReDoS vulnerability pattern as in confluence utils. The nested loop can cause exponential backtracking with malicious HTML input.
Consider adding a maximum iteration limit to prevent denial of service:
99 files reviewed, 3 comments
88d0242 to
a9063f1
Compare
a9063f1 to
0fde557
Compare
* improvement(performance): remove unused source/target indices, add index on snapshot id (#1603) * fix(blog): rename building to blogs with redirect (#1604) * improvement(privacy-policy): updated privacy policy for google (#1602) * updated privacy policy for google * update terms, privacy, and emails to incl address and update verbiage * feat(guardrails): added guardrails block/tools and docs (#1605) * Adding guardrails block * ack PR comments * cleanup checkbox in dark mode * cleanup * fix supabase tools * fix(inference-billing): fix inference billing when stream is true via API, add drag-and-drop functionality to deployed chat (#1606) * fix(inference): fix inference billing when stream is true via API * add drag-and-drop to deployed chat * feat(mistal): added mistral as a provider, updated model prices (#1607) * feat(mistal): added mistral as a provider, updated model prices * remove the ability for a block to reference its own outluts * fixed order of responses for guardrails block * feat(versions): added the ability to rename deployment versions (#1610) * fix(vulns): fix various vulnerabilities and enhanced code security (#1611) * fix(vulns): fix SSRF vulnerabilities * cleanup * cleanup * regen docs * remove unused deps * fix failing tests * cleanup * update deps * regen bun lock
Summary
fix various vulnerabilities and enhanced code security
Type of Change
Testing
Tested manually.
Checklist