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

Process.euid=/Process.egid= don't accept strings #2615

Closed
djberg96 opened this issue Mar 5, 2022 · 4 comments
Closed

Process.euid=/Process.egid= don't accept strings #2615

djberg96 opened this issue Mar 5, 2022 · 4 comments

Comments

@djberg96
Copy link
Contributor

djberg96 commented Mar 5, 2022

Ruby 3.0.2 allows a string for Process.euid=, and will (I assume) do a user lookup in that case:

# OS X
Process.euid = "test" # => can't find user for test (ArgumentError)
Process.euid = "dberger" # => "dberger"

But using Truffleruby 22.0.0.2:

Process.euid = "test" # => undefined method `to_int' for "test":String
Process.euid = "dberger" # => undefined method `to_int' for "dberger":String
@chrisseaton
Copy link
Collaborator

Seems an ancient feature.

ruby/ruby@9bf9b3e

But it's never been properly specified - see here it specifies that it raises an error if not given an integer, but then also specifies that you can't set it to root, using the string, but in order to trigger an unrelated error.

https://github.com/ruby/spec/blob/a0f1a08f38d13cc6491cd1192958046bceceaf39/core/process/euid_spec.rb#L17-L45

Implementation would go here.

def self.euid=(uid)
# the 4 rescue clauses below are needed
# until respond_to? can be used to query the implementation of methods attached via FFI
# atm respond_to returns true if a method is attached but not implemented on the platform
uid = Truffle::Type.coerce_to uid, Integer, :to_int
begin
ret = Truffle::POSIX.setresuid(-1, uid, -1)
rescue NotImplementedError
begin
ret = Truffle::POSIX.setreuid(-1, uid)
rescue NotImplementedError
begin
ret = Truffle::POSIX.seteuid(uid)
rescue NotImplementedError
if Process.uid == uid
ret = Truffle::POSIX.setuid(uid)
else
raise NotImplementedError
end
end
end
end
Errno.handle if ret == -1
uid
end

Should be relatively easy to fix, could be a good beginner project. First thing would be specifications and then an implementation.

@eregon
Copy link
Member

eregon commented Mar 5, 2022

Converting the user name to a uid can be done with Etc.getpwnam(user).uid, which is what fileutils does:

require 'etc'
Etc.getpwnam(user) ? Etc.getpwnam(user).uid : nil

That means it needs require 'etc' but that seems fine since it's a rarely-used method.

@djberg96
Copy link
Contributor Author

djberg96 commented Mar 5, 2022

Note that Process.egid= has the same issue.

@eregon eregon changed the title Process.euid= doesn't accept strings Process.euid=/Process.egid= don't accept strings May 11, 2022
@andrykonchin
Copy link
Member

Looks like the issue is fixed and the PR is merged.

@eregon eregon closed this as completed Jul 6, 2022
@eregon eregon added this to the 22.3.0 milestone Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants