geerlingguy / ansible-role-nodejs

Ansible Role - Node.js
https://galaxy.ansible.com/geerlingguy/nodejs/
MIT License
410 stars 252 forks source link

Security: This role can lead to root compromission #125

Closed maethor closed 3 years ago

maethor commented 3 years ago

In tasks/main.yml this role adds npm.sh in /etc/profile.d, which edit the PATH variable for all users.

As a result, {{ npm_config_prefix }}/bin is prepended to the PATH of the root user. But {{ npm_config_prefix }}/bin may not be owned by root, which gives power to an unprivileged user to override any binary and quickly gain root privileges.

maethor commented 3 years ago

I would suggest at least using the "$HOME" variable, and adding a condition, like what rbenv role is doing: https://github.com/zzet/rbenv/blob/master/templates/rbenv_user.sh.j2

stale[bot] commented 3 years ago

This issue has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark issues as stale.

thbar commented 3 years ago

Somehow related: https://github.com/geerlingguy/ansible-role-nodejs/pull/111 (this is one of the scenarios I wanted to avoid).

stale[bot] commented 3 years ago

This issue is no longer marked for closure.

stale[bot] commented 3 years ago

This issue has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark issues as stale.

geerlingguy commented 3 years ago

See #111 — for convenience I'd rather keep the default so node things work okay (unless there's a better way of doing global installs nowadays?), but I think #111 would offer a good option for those who know what they're doing and want to avoid this pitfall.

stale[bot] commented 3 years ago

This issue is no longer marked for closure.

geerlingguy commented 3 years ago

Workaround implemented in #111.