This repository has been archived by the owner on Apr 11, 2021. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Mitigates a server-side request forgery (SSRF; CWE-918) security vulnerability in the application.
Vulnerability details
SSRF occurs when an attacker can make a request through your server to other external servers. It can hide the attacker's origin IP from the targeted server, allowing for attackers to better cover their tracks. This is because all requests will appear to be coming from the BeePing server, rather than the attacker. SSRF also allows an attacker to enumerate accessible systems. In this particular instance, an attacker would be able to scan internal IP addresses through the BeePing server, which would otherwise be inaccessible from the internet. The BeePing server would essentially act as a proxy between the internet and the local network. In this case, it is somewhat mitigated by the fact that
http.client
only allows for requests using the "http://" and "https://" protocol schemes (and not things like "file://" or "ssh://", which would be far more dangerous; these protocols could be added using a custom transport, but I won't get into that here. See https://golang.org/pkg/net/http/#Transport.RegisterProtocol if you really want to do that). However, due to the fact that any system running BeePing is open to external connections ( #9 ), any system running BeePing would be vulnerable to this attack, opening up internal web servers (on any port, due to the fact that ports can be specified in the URL format withhttp.client
as well) to be discovered by attackers; this information can be leveraged by attackers in further network penetration efforts.Example of successful internal IP address scan:
Mitigation details
Due to the nature of this application, SSRF attacks can never truly be prevented- the application is intended to make outbound requests for users. However, the effects against the organization running an instance of BeePing have been mitigated by removing the ability of a user to scan internal IPv4 and IPv6 addresses (see RFC 1918 for IPv4 and RFC 4193 for IPv6).
What's included
See the following check list from #10:
The IP could not be taken from
conn
, because that would have been too late, and the request would have already occurred (see #10 (comment)). Unfortunately, the nested if statements in the logic are needed, because I don't want to run thevalidate()
method to check for an error if it hasn't been explicitly set in the command line options; otherwise, it'll incorporate more processing than is needed.Cheers,
Aaron