1
0
mirror of https://github.com/veracrypt/VeraCrypt.git synced 2026-05-21 21:30:48 -05:00

Fix undefined behavior in StartElevated stderr read loop (#1550) (#1687)

The read loop that captures stderr from the sudo child process used
`vector<char> buffer(4096); buffer.clear();` followed by
`read(fd, &buffer[0], buffer.capacity())`. This has two instances of
undefined behavior:

1. `operator[](0)` on a vector with `size() == 0` violates the C++
   standard precondition `n < size()`. libstdc++ built with
   `-D_GLIBCXX_ASSERTIONS` aborts the process with:

     stl_vector.h:1128: Assertion '__n < this->size()' failed.

2. `buffer.begin() + bytesRead` on the same empty vector constructs
   an iterator past `end()`, also UB.

`-D_GLIBCXX_ASSERTIONS` is in the default build flags of Arch Linux,
Fedora, and several other distributions. On those systems, the
unprivileged helper process aborts as soon as sudo writes anything
to stderr (a password prompt, a 'user is not in the sudoers file'
error, etc.). The main process then sees EOF on the service output
pipe, and throws `InsufficientData`, which renders to the user as
'Not enough data available'. A second mount attempt fails at
`File::Write` because the helper is dead and the pipe is broken,
producing the bare message 'VeraCrypt::File::Write:395'.

Fix by replacing `buffer` with a plain `char[4096]` and using
`reserve(4096)` on `errOutput` to preserve the original
pre-allocation intent. No behavioral change on systems where the UB
happened to work; aborts are eliminated on systems where the
assertions fire.

Reported-by: multiple users, see veracrypt/VeraCrypt#1550,
              veracrypt/VeraCrypt#1446, veracrypt/VeraCrypt#844

Co-authored-by: rabbit <rabbit@github>
This commit is contained in:
curious-rabbit
2026-04-23 14:03:20 +02:00
committed by GitHub
parent 357ce6bd7a
commit e7188c96a4
+5 -5
View File
@@ -503,9 +503,9 @@ namespace VeraCrypt
throw_sys_if (fcntl (outPipe->GetReadFD(), F_SETFL, O_NONBLOCK) == -1); throw_sys_if (fcntl (outPipe->GetReadFD(), F_SETFL, O_NONBLOCK) == -1);
throw_sys_if (fcntl (errPipe.GetReadFD(), F_SETFL, O_NONBLOCK) == -1); throw_sys_if (fcntl (errPipe.GetReadFD(), F_SETFL, O_NONBLOCK) == -1);
vector <char> buffer (4096), errOutput (4096); char buffer[4096];
buffer.clear (); vector <char> errOutput;
errOutput.clear (); errOutput.reserve (4096);
Poller poller (outPipe->GetReadFD(), errPipe.GetReadFD()); Poller poller (outPipe->GetReadFD(), errPipe.GetReadFD());
int status, waitRes; int status, waitRes;
@@ -518,10 +518,10 @@ namespace VeraCrypt
ssize_t bytesRead = 0; ssize_t bytesRead = 0;
foreach (int fd, poller.WaitForData (timeout)) foreach (int fd, poller.WaitForData (timeout))
{ {
bytesRead = read (fd, &buffer[0], buffer.capacity()); bytesRead = read (fd, buffer, sizeof (buffer));
if (bytesRead > 0 && fd == errPipe.GetReadFD()) if (bytesRead > 0 && fd == errPipe.GetReadFD())
{ {
errOutput.insert (errOutput.end(), buffer.begin(), buffer.begin() + bytesRead); errOutput.insert (errOutput.end(), buffer, buffer + bytesRead);
if (bytesRead > 5 && bytesRead < 80) // Short message captured if (bytesRead > 5 && bytesRead < 80) // Short message captured
timeout = 200; timeout = 200;