Skip to content
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

Security: lack of input validation in APIs leads to command injection #23

Closed
retpoline opened this issue Jan 31, 2021 · 5 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@retpoline
Copy link

retpoline commented Jan 31, 2021

For example, making GET requests such as the following results in arbitrary commands being executed on the server, outside of the intended docker process only. This is otherwise known as a remote command injection bug (especially since app.js listens on the network interface by default, accessible by anyone on the network, even though the intended access is localhost).

https://github.com/rakibtg/docker-web-gui/blob/master/backend/controllers/ContainerController.js#L40

/api/container/command?container=&command=;uname%20-a
/api/image/command?image=&command=;id

For example executing the id command may return:

uid=1000(test) gid=1000(test) groups=1000(test),133(docker)

Some of the other APIs seem vulnerable to injection as well.

Recommended easiest fix would be the following

  1. allow only alphanumeric characters in the parameters (strings that actually look like container and image names)
  2. have a list of allowed commands such as start/stop, etc. Ensure this simple validation routine validates the user input before concatenating parameters and calling Terminal(cmd) or in terminal.js itself. Return a "bad parameter" error message if the validation doesn't pass.

Additional references for hardening the code

@rakibtg
Copy link
Owner

rakibtg commented Feb 3, 2021

Thanks @retpoline for the details and the references. Will look into it asap!

@retpoline
Copy link
Author

Hey @rakibtg,

Just following up here-- did you get a chance to look into hardening the APIs to fix the command injection security issues?

@rakibtg
Copy link
Owner

rakibtg commented Feb 20, 2021

@retpoline Hi, sorry for the delay. Sadly, I will not available till this Tuesday!

@rakibtg rakibtg self-assigned this Feb 27, 2021
@rakibtg rakibtg added the bug Something isn't working label Feb 27, 2021
@rakibtg
Copy link
Owner

rakibtg commented Feb 28, 2021

Fixed security issue with terminal by introducing controlled method that execute queries in

exports.safeTerminal = {
installModules: async (backendPath) => {
await Terminal(`cd ${backendPath} && npm install`);
},
serve: async (backendPath) => {
await Terminal(`cd ${backendPath} && node index.js`);
},
allContainers: () => Terminal(`docker ps -q -a`),
inspectContainer: async (id) => {
if (isValidId(id)) {
return Terminal(`docker container inspect ${id}`);
} else {
throw new Error("The container id is invalid");
}
},
generic: async (task, id) => {
if (!isValidString(task)) {
throw new Error("The task command is invalid.");
}
if (!isValidId(id)) {
throw new Error("The container id is invalid");
}
return Terminal(`docker container ${task} ${id}`);
},
logs: async (id) => {
if (!isValidId(id)) {
throw new Error("The container id is invalid");
}
return Terminal(`docker container logs ${id} --tail 1500`);
},
stats: () =>
Terminal(
`docker container stats --no-stream --format '{"id": "{{.ID}}", "cpu_percentage": "{{.CPUPerc}}", "memory_usage": "{{.MemUsage}}", "network_io": "{{.NetIO}}"}'`
),
prune: (pruneType) => {
if (!isValidString(pruneType)) {
throw new Error("The entity type is not valid");
}
return Terminal(`docker ${pruneType} prune -f`);
},
containerLs: () => Terminal(`docker container ls --format '{{json .}}'`),
formattedImages: () =>
Terminal(
`docker images --format '{"ID": "{{.ID}}", "Tag": "{{.Tag}}", "CreatedSince": "{{.CreatedSince}}", "Size": "{{.Size}}", "VirtualSize": "{{.VirtualSize}}", "Repository": "{{.Repository}}"}'`
),
singleImage: (task, id) => {
if (!isValidString(task)) {
throw new Error("The task command is invalid.");
}
if (!isValidId(id)) {
throw new Error("The image id is invalid");
}
if (task == "run") {
return Terminal(`docker ${task} ${id}`);
} else {
return Terminal(`docker image ${task} ${id}`);
}
},
};

Let me know your opinion! Thanks.

@rakibtg rakibtg closed this as completed Mar 1, 2021
@abergmann
Copy link

CVE-2021-27886 was assigned to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants