erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.39k stars 2.96k forks source link

os:cmd() invokes access() syscall as part of detecting which shell to use #8944

Open yarisx opened 1 month ago

yarisx commented 1 month ago

Describe the bug Since OTP25 the os:cmd() function via its helper os:mk_cmd() tries to determine which shell to use (to accommodate to Android's non-standard shell placement) by calling file:read_file_info(), which in turn invokes access() syscall with R_OK and W_OK flags. This causes issues in some secured environments where only read access to the shell is allowed. Moreover reading the whole inode of data only to check that the file exists looks unnecessary.

To Reproduce In Linux: from Erlang shell call os:cmd("ls") while using strace -f -e trace=access on the Erlang VM process.

Expected behavior os:cmd() should check the location of the system shell without reading a lot of info from the filesystem and triggering security modules like SELinux and AppArmor.

Affected versions OTP25 and further

Additional context It seems to be possible to use the access() syscall with F_OK flag to just check whether the file exists or not. Probably it would be beneficial to have a separate file:check_access/2 function to check access rights on the file without reading all other data about it or opening (and immediately closing) it.

garazdawi commented 3 weeks ago

Another approach would be to add a configuration parameter to kernel, that lets the user decide which shell to use. Doing a config lookup should not be very expensive in the big scheme of things.

What do you think?

yarisx commented 3 weeks ago

Yes, the config option would work to select the shell. But our wish for file:check_access/2 still stays :-) We often check read/write access to files (logs, credentials etc) but we are not interested in other metadata for those files. Probably it should be a matter of another PR though.

garazdawi commented 3 weeks ago

Yes, the config option would work to select the shell.

Will you send a PR?

But our wish for file:check_access/2 still stays :-) We often check read/write access to files (logs, credentials etc) but we are not interested in other metadata for those files. Probably it should be a matter of another PR though.

The problem with file:check_access/2 is that on Windows there is no equivilant check that makes sense (that we know of).