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

Crystal::System::User#from_*? et al. don't work if required buffer size greater than initial buffer size #14619

Closed
BlobCodes opened this issue May 23, 2024 · 1 comment · Fixed by #14622
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:system

Comments

@BlobCodes
Copy link
Contributor

Bug Report

While looking at #14614, I noticed that all methods using System.retry_with_buffer do not work if the required buffer size to store strings in is greater than the initial buffer size provided.

Here's an example (Group#from_name?):

    grp = uninitialized LibC::Group
    grp_pointer = pointerof(grp)
    System.retry_with_buffer("getgrnam_r", GETGR_R_SIZE_MAX) do |buf|
      LibC.getgrnam_r(groupname, grp_pointer, buf, buf.size, pointerof(grp_pointer))
    end

LibC.getgrnam_r (and the other relevant methods) set the value of the last argument to NULL if an error occured:

On success, getgrnam_r() and getgrgid_r() return zero, and set *result to grp. If no matching group record was found, these functions return 0 and store NULL in *result. *In case of error, an error number is returned, and NULL is stored in result.

However, the last argument provided is pointerof(grp_pointer), so grp_pointer will be set to null if the buffer was to small - but this variable is never reset and also used as the second argument to getgrnam_r (where the group data should be stored in).

A quick fix would be to replace the second argument (grp_pointer) with pointerof(grp), so the pointer is never null.
With this fix, there's also no reason to define the value of grp_pointer, it could just be replaced by a out grp_pointer inside the function call.

TL;DR: After one iteration of retry_with_buffer, grp_pointer = null, which causes a segfault in very special circumstances.


With this bash script, you can get your very own segfault:

long_dir_name="/tmp/"
for ((i=0; i<20000; i++)); do long_dir_name+="a"; done

sudo useradd --base-dir="$long_dir_name" baduser

crystal eval '                                  
  require "system/user"
  System::User.find_by(name: "baduser")
'
@BlobCodes BlobCodes added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label May 23, 2024
@straight-shoota
Copy link
Member

Oh yeah, the double use of grp_pointer as both input and output argument is really biting us here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants