feat(tests): add unit tests for validation functions in docker-contai…#3735
Conversation
…ner-logs - Introduced tests for isValidTail, isValidSince, isValidSearch, and isValidContainerId functions to ensure proper validation and security against command injection. - Updated docker-container-logs to utilize these validation functions, enhancing input handling for WebSocket connections.
Additional Comments (2)
|
…haracters - Updated the regex in the isValidSearch function to limit valid characters, improving input validation and security against potential injection attacks.
|
@greptile review |
apps/dokploy/server/wss/utils.ts
Outdated
There was a problem hiding this comment.
Critical: allows $, `, ', and ( which enable command injection. Since search is inserted into shell commands (line 99 uses double quotes where $ and ` execute; line 153 uses single quotes where ' breaks out), this creates a remote code execution vulnerability.
| export const isValidSearch = (search: string): boolean => { | |
| return /^[\x20-\x21\x23-\x25\x27-\x28\x2A-\x3A\x3D\x3F-\x5B\x5D-\x7B\x7D-\x7E]{0,500}$/.test( | |
| search, | |
| ); | |
| export const isValidSearch = (search: string): boolean => { | |
| return /^[a-zA-Z0-9\s._-]{0,500}$/.test(search); | |
| }; |
Additional Comments (1)
|
- Enhanced the isValidSearch function to restrict allowed characters to alphanumeric, space, dot, underscore, and hyphen, preventing command injection vulnerabilities. - Updated unit tests to reflect the new validation rules and ensure comprehensive coverage against potential injection attacks.
…ner-logs
What is this PR about?
Please describe in a short paragraph what this PR is about.
Checklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
Screenshots (if applicable)
Greptile Summary
Added validation functions and unit tests for docker container log parameters (
tail,since,search,containerId) to prevent command injection attacks. However, theisValidSearchregex still allows critical shell metacharacters ($,`,',(), and these parameters are directly embedded in shell commands executed via SSH and locally.Critical security issues:
isValidSearchallows$,`,', and(which enable command substitution$(cmd)and`cmd`execute'can break out of quotingOther findings:
isValidTail,isValidSince, andisValidContainerIdvalidations are strongConfidence Score: 0/5
isValidSearchfunction allows shell metacharacters ($,`,',() that enable command injection. These are directly exploitable in the SSH path (line 99, double quotes) and local path (line 153, single quotes) where user input is embedded in shell commands. An attacker can execute arbitrary commands by sending malicious search parameters.apps/dokploy/server/wss/utils.tsandapps/dokploy/server/wss/docker-container-logs.tscontain exploitable command injection vulnerabilitiesLast reviewed commit: 33c3a4e