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.

Sign up for GitHub

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

Jump to bottom

Add winpty_get_console_process_list #130

Merged
merged 3 commits into from
Oct 8, 2017
Merged

Add winpty_get_console_process_list #130

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1424696
Add winpty_get_console_process_list
the-ress Sep 16, 2017
39fe32b
Fix for edge case when process list changes between GetConsoleProcess…
the-ress Oct 3, 2017
f678472
Replaced overflow check
the-ress Oct 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/agent/Agent.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,16 @@ void Agent::handleGetConsoleProcessListPacket(ReadBuffer &packet)

auto processList = std::vector<DWORD>(64);
auto processCount = GetConsoleProcessList(&processList[0], processList.size());
if (processList.size() < processCount) {
processList.resize(processCount);

// The process list can change while we're trying to read it
while (processList.size() < processCount) {
// Multiplying by two caps the number of iterations
const int newSize = processList.size() * 2;
if (newSize <= processList.size()) { // Ensure we fail when new size overflows
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this code were to overflow, processList.size() would be 0x40000000, and newSize would be -0x80000000 (i.e. INT_MIN). newSize would be greater than processList.size() because it would be converted from int to size_t for the comparison.

I'd prefer to replace "const int newSize = ... ; ... if { ... }" with:

const auto newSize = std::max<DWORD>(processList.size() * 2, processCount);

If you do, also add #include <algorithm> to the top.

Overflow shouldn't ever happen. If it somehow does, then there is one code path handling it -- std::vector::resize throws std::bad_alloc.

processCount = 0;
break;
}
processList.resize(newSize);
processCount = GetConsoleProcessList(&processList[0], processList.size());
}

Expand Down