Dokploy
When adding the registry, the parameters were not escaped, leading to a bug and security vulnerability
To Reproduce
- Add a external registry.
- input the correct password with special symbols.(e.g. '123$456')
- click the Test Registry or Create button
Current vs. Expected behavior
Current: It failed because of an incorrect password. Expected: It works.
Provide environment information
Dokploy Version: v0.9.0
Which area(s) are affected? (Select all that apply)
Docker
Additional context
The issue here lies in the fact that the password and other inputs are directly passed as parameters to the execute command without any escaping.
const loginCommand = `echo ${input.password} | docker login ${input.registryUrl} --username ${input.username} --password-stdin`; await execAsync(loginCommand);
Therefore, when the password contains special characters, the registry cannot be added properly. bytheway, wrapping the password in single quotes manually allows it to be added successfully.
So, there is also a command injection security vulnerability: the parameters are not securely escaped, which means arbitrary commands could be executed in a privileged Docker container, potentially allowing an escape to the host machine.
Although this requires access to the admin panel.
Well I wouldn't really consider it a vulnerability, since they need access to the dashboard to be able to manipulate, and if they access the dashboard they could do almost anything, I will investigate some solution to escape characters or what would be the best solution for this.
If anyone has an idea how to solve this it would be great, I was researching but it seems that there is no exact way to remove extraneous characters, there will always be a way to bypass the validation
If anyone has an idea how to solve this it would be great, I was researching but it seems that there is no exact way to remove extraneous characters, there will always be a way to bypass the validation
Perhaps we can consider using execfile or spawn instead of exec for command execution, as both methods allow separating the command from its arguments.
In most cases, parameterized ensures a higher level of security.
For specific scenarios, additional validation checks on the input parameters can be implemented to address potential risks.
e.g.
await execFileAsync('docker', [ 'login', input.registryUrl, '--username', input.username, '--password-stdin' ], { input: input.password });
I understand it might not be a simple task, but perhaps we can start by attempting to replace the functionality in the code where the command parameters are provided by user input.
Just ran into this issue. When using harbor registry, robot accounts are prefixed with robot$. In bash, anything prefixed with $ is expanded to the contents of that environment variable. In my case, it was expanding robot$dokploy, replacing $dokploy with an empty string since that env variable doesn't exist.
A quick workaround was to escape the special character with \, so entering robot\$dokploy instead.
I think the solution @xinghejd mentioned is a great way forward. child_process.exec spawns a shell and runs the requested command within that shell, while child_process.{spawn,execfile} do not spawn shells, and directly invoke the command, passing in the args untouched.
