Skip to content

breaking change: Remove ad-hoc setuid/setgid bindings#822

Open
squeek502 wants to merge 1 commit intoluvit:masterfrom
squeek502:escape-from-setgid-setuid
Open

breaking change: Remove ad-hoc setuid/setgid bindings#822
squeek502 wants to merge 1 commit intoluvit:masterfrom
squeek502:escape-from-setgid-setuid

Conversation

@squeek502
Copy link
Copy Markdown
Member

@squeek502 squeek502 commented Mar 30, 2026

These functions aren't coming from libuv, they aren't portable (to the point of the function bindings being wrapped in a #ifndef _WIN32), and there's proper support for setgid/setuid in the libuv process spawning API.

These functions are also seemingly unused except by a single personal project for which the bindings were added in the first place. The ad-hoc getuid/getgid functions do see some use in neovim plugins, so they remain but it's now recommended to use the proper os_get_passwd function from libuv instead.

There's also some potential security issues around setuid/setgid that we aren't dealing with correctly and there's no good reason to take on that responsibility (closes #341).


Alternative to #817

docs/docs.lua Outdated
},
warnings = {
[[
This is a legacy function that may be removed in the future. It's recommended to use `uv.os_get_passwd()` instead.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This is a legacy function that may be removed in the future. It's recommended to use `uv.os_get_passwd()` instead.
Deprecated. Use `uv.os_get_passwd()` instead.

I would phrase this even more strongly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, as far as I can tell, and by all means, a security threat at best, or an active vulnerability at worst, as I can understand? so yeah I agree

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also add when (i.e., what luv version) it will be removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, if not for the Neovim usage, I believe we would have removed it as soon as we could.

Copy link
Copy Markdown

@clason clason Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We break enough stuff ourselves, don't worry too much on our account ;) The alternative is also easy to switch to and for us to document.

(But I appreciate the thought!)

I think it's good to have a full version where users can switch, i.e., introduce the warning for the 1.53 release and remove at 1.54. If you want to fast-track it, make this 1.52.2 and 1.53.0, respectively.

Copy link
Copy Markdown
Member Author

@squeek502 squeek502 Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bilal2453 to be clear, there's no security issue with the get functions AFAIK, that only pertains to the set functions which are removed entirely in this PR.

I think "deprecated" is probably a better description for the get functions.

These functions aren't coming from libuv, they aren't portable (to the point of the function bindings being wrapped a #ifndef _WIN32), and there's proper support for setgid/setuid in the libuv process spawning API.

These functions are also seemingly unused except by a single personal project for which the bindings were added in the first place. The ad-hoc getuid/getgid functions do see some use in neovim plugins, so they remain but it's now recommended to use the proper `os_get_passwd` function from libuv instead.

There's also some potential security issues around setuid/setgid that we aren't dealing with correctly and there's no good reason to take on that responsibility (closes luvit#341).
@squeek502 squeek502 force-pushed the escape-from-setgid-setuid branch from f582cf5 to 5444fd5 Compare March 30, 2026 20:32
Copy link
Copy Markdown
Member

@truemedian truemedian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more on board with removing these than with trying to ensure people don't use them incorrectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Library calls setuid and/or setgid without setgroups or initgroups

4 participants