Security issues in http.go, buddyinfo.go, ipvs.go, net_dev.go, Logutil.go

[freeware83]

What is the version of library used?

v0.9.2

What is the issue faced?

We are using the prometheus golang library v0.9.2 for exposing prometheus metrics from the Golang project. All the dependency packages including prometheus client_golang library are resolved in the vendor folder as per the go lang feature. We do have a static code analysis for security vulnerability using checkmarx which scans the code for security vulnerability. It has flagged the prometheus library files for the following security issues reasons

Filename	Issue	Source Line number
http.go	XSS	70 & 125
buddyinfo.go	Denial of service resource exhaustion	64
ipvs.go	Denial of service resource exhaustion	167
net_dev.go	Denial of service resource exhaustion	86
Logutil.go	Server-Side Request Forgery	109

What is the Impact?

We are not able to proceed using this library for our metrics.

Resolution

Appreciate if this could be updated in the next version

[beorn7]

Thanks for sharing your results. Could you let us know in which paths those files are located? There are, for example, many files called http.go in the codebase and its dependencies.

[freeware83]

Below are the path

github.com\prometheus\client_golang\prometheus\http.go
github.com\prometheus\client_golang\prometheus\promhttp\http.go
github.com\prometheus\procfs\buddyinfo.go
github.com\prometheus\procfs\ipvs.go
github.com\prometheus\procfs\net_dev.go
github.com\prometheus\procfs\Logutil.go

[beorn7]

github.com\prometheus\client_golang\prometheus\http.go github.com\prometheus\client_golang\prometheus\promhttp\http.go

The flagged line seems to be contentType := expfmt.Negotiate(req.Header). Could you explain how that would result in an XSS vulnerability?

github.com\prometheus\procfs\buddyinfo.go

L60 reads from /proc/buddyinfo with scanner.Text(). I assume this was flagged because it would exhaust resources if /proc/buddyinfo contains sufficiently long lines. However, the same could be said about any usage of scanner.Text(). I don't see a vulnerability here as /proc/buddyinfo is not under control of a possible attacker (or if it is, the attacker would have much easier ways to exhaust the resources of the system).

github.com\prometheus\procfs\ipvs.go github.com\prometheus\procfs\net_dev.go

Same thing in both files. Your code analyzer apparently flags any call of scanner.Text().

github.com\prometheus\procfs\Logutil.go

I cannot find that file.

[ashidhull]

Checkmarx pointed out all these risks and since that is an important step for Go deployment it is causing the builds to fail

[wy100101]

I don't think there is XSS vulnerability. It looks like Checkmarx is missing the fact that Negotiate() is sanitizing the user provide content type.

[beorn7]

Checkmarx pointed out all these risks and since that is an important step for Go deployment it is causing the builds to fail

But I fail to understand what the nature of the issue is. Could you provide context? Otherwise, there is no way to fix the security issue.

[freeware83]

We are using prometheus golang library for our metrics. Checkmarx thinks that there is un sanitized data consumed in http.go which would cause XSS. Checkmarx is part of our CICD.

Quoting From the check marx report for XSS. I can provide for remaining if required.

The application's UninstrumentedHandler embeds untrusted data in the generated output with Set, at line 62 of github.com/prometheus/client_golang/prometheus/http.go. This untrusted data is embedded straight into the output without proper sanitization or encoding, enabling an attacker to inject malicious code into the output. The attacker would be able to alter the returned web page by simply providing modified data in the user input Header, which is read by the UninstrumentedHandler method at line 62 of github.com/prometheus/client_golang/prometheus/http.go. This input then flows through the code straight to the output web page, without sanitization. This can enable a Reflected Cross-Site Scripting (XSS) attack.

Would like to know if these security issues are something we should be concerned about? Is that something that can be fixed in future releases?

[wy100101]

The only Set() I see is at line 71 header.Set(contentTypeHeader, string(contentType)) contentType come from line 69 contentType := expfmt.Negotiate(req.Header) expfmt.Negotiate() only uses req.Header for a case statement to decide which of the supported content-types to use in the response. So it isn't actually embedding anything provided by the client in the response.

I don't think there is a XSS here.

[juliusv]

Yeah, I don't know Checkmarx, but it sounds like a security scanner gone wild that perhaps produces a list of suggestions for further investigation, but that doesn't necessarily indicate that there are actual vulnerabilities present. I also cannot find Logutil.go anywhere.

I'm going to close this as a false positive for now. Please let us know if this is wrong and you see an actual issue here somewhere.