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

VeraCrypt (gui, Linux) thows a message: "Failed to obtain administrator privileges: invalid option provided" with sudo-rs #1331

Open
id3v1669 opened this issue Apr 18, 2024 · 3 comments
Labels

Comments

@id3v1669
Copy link

id3v1669 commented Apr 18, 2024

Expected behavior

Run as intended

Observed behavior

Throws a popup window with "Failed to obtain administrator privileges: invalid option provided"

Steps to reproduce

  1. Use sudo-rs instead of regular sudo
  2. Try to perform any action that requires priveledges

Your Environment

VeraCrypt version: 1.26.7
Operating system and version: Linux (NixOS unstable)
System type: 64bit

#Why happes
Because sudo-rs has fewer options and doesn't provide -p flag

#Possible fix
Replace:

const char *args[] = { "sudo", "-S", "-p", "", appPath.c_str(), TC_CORE_SERVICE_CMDLINE_OPTION, nullptr };

With:

const char *args[] = { "sudo", "-S", appPath.c_str(), TC_CORE_SERVICE_CMDLINE_OPTION, nullptr };

And I do not understand why it is there in the first place as it uses empty string as argument anyway, but seems to work with both sudo and sudo-rs
Tested on:
sudo - 1.9.15p5
sudo-rs - 0.2.2

@Jertzukka
Copy link
Contributor

The -p flag for sudo indicates the text prompt that will be asked from the user when they're requested for their password. This will be done even with the -S flag which changes the behaviour so that the prompt will actually be printed to stderr.

In CoreService::StartElevated the stderr of this for forked sudo is redirected into errPipe, and when you call sudo without -p argument, the environment value of SUDO_PROMPT is printed into this forks stderr as in errPipe in our case. This is then polled for and read later and inserted into errOutput. Next it is checked that if errOutput is empty (it is not because SUDO_PROMPT is printed here now), and we attempt to deserialize the error, and silently failing.

Next, if the exitCode is not 0, the errOutput is set as the strErrOutput which is then added as an argument in ElevationFailed exception. This will cause your SUDO_PROMPT environment variable to be printed into your error message if the elevation fails for some reason, such as you giving the wrong password.

Not only would removing the option add either irrelevant prompts or user defined input into the error messages, it might also be possible to handcraft a specific SUDO_PROMPT value which deserializes to a valid Exception, which is then thrown.

What I would recommend is requesting the addition of this feature/implementation on the sudo-rs GitHub issues.

@id3v1669
Copy link
Author

The -p flag for sudo indicates the text prompt that will be asked from the user when they're requested for their password. This will be done even with the -S flag which changes the behaviour so that the prompt will actually be printed to stderr.

In CoreService::StartElevated the stderr of this for forked sudo is redirected into errPipe, and when you call sudo without -p argument, the environment value of SUDO_PROMPT is printed into this forks stderr as in errPipe in our case. This is then polled for and read later and inserted into errOutput. Next it is checked that if errOutput is empty (it is not because SUDO_PROMPT is printed here now), and we attempt to deserialize the error, and silently failing.

Next, if the exitCode is not 0, the errOutput is set as the strErrOutput which is then added as an argument in ElevationFailed exception. This will cause your SUDO_PROMPT environment variable to be printed into your error message if the elevation fails for some reason, such as you giving the wrong password.

Not only would removing the option add either irrelevant prompts or user defined input into the error messages, it might also be possible to handcraft a specific SUDO_PROMPT value which deserializes to a valid Exception, which is then thrown.

What I would recommend is requesting the addition of this feature/implementation on the sudo-rs GitHub issues.

So do I get this right:

  1. using regular sudo without -p flag is unsafe and might cause trouble
  2. using sudo-rs without -p flag is safe, but it might cause inappropriate output to user

@Jertzukka
Copy link
Contributor

Jertzukka commented Apr 19, 2024

So do I get this right:

1. using regular `sudo` without `-p` flag is unsafe and might cause trouble

2. using `sudo-rs` without `-p` flag is safe, but it might cause inappropriate output to user
  1. Removing -p flag with normal sudo will cause incorrect error messages and your SUDO_PROMPT being deserialized as an exception.

  2. What I quickly read the help from sudo-rs, it seems that their -S implementation doesn't say that it would redirect prompts into stderr, so if that is correct, sudo-rs wouldn't affect the error messages but it might instead just write it to stdout. This would mean the prompt is written to the ServiceOutputStream which is then used again for deserialization.

The thing is most of these pipes between these processes and their file descriptors are in use for inter-process communication and introducing random text into any of the pipes is likely to cause some issue. Both of these will have some side effects but it's hard to evaluate how noticeable they'll be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants