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

Request lowest access privilege required in Windows OpenProcess calls #50

Merged
merged 1 commit into from
Oct 13, 2016

Conversation

andrewkroh
Copy link
Member

Gosigar requests PROCESS_ALL_ACCESS permission but the operation that it performs, GetProcessMemoryInfo, requires only PROCESS_QUERY_INFORMATION and PROCESS_VM_READ. gosigar should only request the permissions that it requires.

In other parts of gosigar it requests PROCESS_QUERY_INFORMATION, but in Windows Vista and newer there is a more limited privilege we can request called PROCESS_QUERY_LIMITED_INFORMATION. This can be used for GetProcessTimes, GetProcessImageFileName, and GetExitCodeProcess. It cannot be used for OpenProcessToken.

Gosigar requests PROCESS_ALL_ACCESS permission but the operation that it performs, GetProcessMemoryInfo, requires only PROCESS_QUERY_INFORMATION and PROCESS_VM_READ. gosigar should only request the permissions that it requires.

In other parts of gosigar it requests PROCESS_QUERY_INFORMATION, but in Windows Vista and newer there is a more limited privilege we can request called PROCESS_QUERY_LIMITED_INFORMATION. This can be used for GetProcessTimes, GetProcessImageFileName, and GetExitCodeProcess. It cannot be used for OpenProcessToken.
@codecov-io
Copy link

codecov-io commented Oct 13, 2016

Current coverage is 70.60% (diff: 69.56%)

Merging #50 into master will decrease coverage by 0.13%

@@             master        #50   diff @@
==========================================
  Files            10         10          
  Lines          1080       1099    +19   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            764        776    +12   
- Misses          257        262     +5   
- Partials         59         61     +2   

Powered by Codecov. Last update c476970...ed008d7

Copy link

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -0,0 +1,17 @@
# Change Log
Copy link

Choose a reason for hiding this comment

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

Thx for adding the changelog

@ruflin ruflin merged commit 15322f7 into elastic:master Oct 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants