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

Security Vulnerability in Munin-node-win32 Service. #78

Open
0xneel opened this issue Jul 9, 2019 · 5 comments
Open

Security Vulnerability in Munin-node-win32 Service. #78

0xneel opened this issue Jul 9, 2019 · 5 comments

Comments

@0xneel
Copy link

0xneel commented Jul 9, 2019

First of all, thanks for keeping this great open source monitoring tool alive.

I found one security vulnerability in Munin-node-win32 Service. Maybe you guys are aware of it, I am not sure, but I haven’t found relevant CVE related to this issue.

Please find this article explaining details of the vulnerability with steps (along with snapshots) to reproduce.

If this is known vulnerability and corresponding CVE is present then please let me know the CVE. I have reserved a CVE number for this vulnerability. In that case, I will close this CVE request.
If this is unknown issue to you then please confirm it. Once the issue is fixed, we can publish this CVE, or if you want, we can publish this before the fix.

@sumpfralle
Copy link
Collaborator

Thank you for reporting this issue!

I am not using munin-node-win32, thus I have some questions:

  • The article mentions the older version 1.6.0 instead of the more recent 1.6.1. Does this latest (even though quite dated) release show the same behaviour?
  • Are unprivileged users allowed to write to C:\Program Files\ by default?

Is the kind of "execution guessing" that you described (conditional tokenizing based on the existence of files) really Windows' documented behavior of that service definition field? That sounds like an embarrassingly awful concept ...

@0xneel
Copy link
Author

0xneel commented Jul 14, 2019

I tested on an executable https://github.com/downloads/munin-monitoring/munin-node-win32/munin-node-win32-1.6.1.0-custom_perfmon-installer.exe It says version 1.6.1.0 (uploaded on 2012). I am not sure why it says version 1.6.0 (Beta) after installation. If there is any recent executable then please give me a downloadable link, I will run the same checks on it.

The unprivileged user includes both People and Service Users. I have updated the above-linked document for more clarity. 'Service User' accounts usually have permissions to create a file in 'C' directory. This could allow a malicious user with local access to execute code with administrative privileges.

Regarding 'execution guessing' Windows behavior, I am not sure whether this is documented by Microsoft or not. But Windows takes BINARY_PATH_NAME as it is and processes it. This is one of the most common Privilege Escalation vulnerability found in the wild for Windows environment. (Which most of the developer's do not know). You can find more details on Unquoted Paths Local Privilege Escalation vulnerability here: https://attack.mitre.org/techniques/T1034/ (Read paragraph Titled 'Unquoted Paths')

Whenever registering a service, developers need to specify Binary Path surrounded with double quotes by doing something like below:
"\"C:\Program Files\Munin Node for Windows\munin-node.exe\""

@sumpfralle
Copy link
Collaborator

sumpfralle commented Jul 14, 2019

I tested on an executable https://github.com/downloads/munin-monitoring/munin-node-win32/munin-node-win32-1.6.1.0-custom_perfmon-installer.exe It says version 1.6.1.0 (uploaded on 2012). I am not sure why it says version 1.6.0 (Beta) after installation. If there is any recent executable then please give me a downloadable link, I will run the same checks on it.

In this case I assume, that you used the most recent version.

The unprivileged user includes both People and Service Users. I have updated the above-linked document for more clarity. 'Service User' accounts usually have permissions to create a file in 'C' directory. This could allow a malicious user with local access to execute code with administrative privileges.

Thank you for the clarification.

Regarding 'execution guessing' Windows behavior, I am not sure whether this is documented by Microsoft or not. But Windows takes BINARY_PATH_NAME as it is and processes it.

<unrelated rant>
It is a shame, that the BINARY_PATH_NAME is not just processed as it is (being one token if quoted or multiple tokens if unquoted). Instead the following iterative guessing is applied:

  • check whether the shortest token matches a filename (probably even tolerant regarding an existing or missing .com, .bat or .exe extension)
  • as long as no match is found: assume that the path name of the executable was supposed to include the next token and try this combined path again

I understand, that such a behavior have been useful for providing a high level of backwards compatibility in the Windows environment. But unsurprisingly such non-deterministic command execution is a welcome source of abuse.
</unrelated rant>

Whenever registering a service, developers need to specify Binary Path surrounded with double quotes by doing something like below:
"\"C:\Program Files\Munin Node for Windows\munin-node.exe\""

Thank you for the explanation!

Thus I assume that the handling of szFilePath in https://github.com/munin-monitoring/munin-node-win32/blob/master/src/core/Service.cpp#L188 should be adjusted to prevent this.

(I do not maintain munin-node-win32 - thus I am just commenting)

@munin-monitoring munin-monitoring deleted a comment from sumpfralle Jan 29, 2020
@singularcitrus
Copy link
Contributor

Please tell me if it is still an issue on the newest version....

@ironoxid
Copy link

Yes it is.

Maybe it is useful to remove Run and UninstallRun sections from installer files.
From a security perspective it would be better if sysadmin manually creates service and firewall rules.
Add/remove a service or a firewall rule must not be a problem for a sysadmin and a manual operation (with some examples) gives to him more control (for example accepting connections only from munin host by MasterAddress on MuninNode configuration section is a good option and enforcing it by open 4949 port only to munin node host by firewall policy it's better).

Workaround to fix this security issue

  1. Stop current munin-node instance
    sc stop munin-node
  2. Remove current munin-node service
    sc delete munin-node
  3. Create a new fixed munin-node service
    sc create munin-node-fixed binPath="\"C:\Program Files (x86)\Munin Node\munin-node.exe\"" start=auto DisplayName="Munin Node Security Fixed Service"
  4. Start the new service
    sc start munin-node-fixed

Please note that binPath may vary, adjust it according to your system.
Please note that if you prefer you can simply change binPath on the default installed service:
sc config munin-node binPath="\"C:\Program Files (x86)\Munin Node\munin-node.exe\""

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

No branches or pull requests

4 participants