[DRAFT][DO NOT MERGE] Remote hooks golang scripts#39
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
liukatkat
changed the title
![semgrep-zcs-prod-semgrep[bot]](https://avatars.githubusercontent.com/u/29760937?s=60&v=4){kind=small}
| client := &http.Client{Timeout: 5*time.Minute + 5*time.Second} | ||
|
|
||
| for { | ||
| req, err := http.NewRequest("POST", url, bytes.NewReader(body)) |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Server-Side-Request-Forgery (SSRF) exploits backend systems that initiate requests to third
parties.
If user input is used in constructing or sending these requests, an attacker could supply
malicious
data to force the request to other systems or modify request data to cause unwanted actions.
Ensure user input is not used directly in constructing URLs or URIs when initiating requests
to third party
systems from back end systems. Care must also be taken when constructing payloads using user
input. Where
possible restrict to known URIs or payloads. Consider using a server side map where key's are
used to return
URLs such as https://site/goto?key=1 where {key: 1, url: 'http://some.url/', key: 2, url: 'http://...'}.
If you must use user supplied input for requesting URLs, it is strongly recommended that the
HTTP client
chosen allows you to customize and block certain IP ranges at the network level. By blocking
RFC 1918
addresses or other network address ranges, you can limit the severity of a successful SSRF
attack. Care must
also be taken to block certain protocol or address formatting such as IPv6.
If you can not block address ranges at the client level, you may want to run the HTTP client
as a protected
user, or in a protected network where you can apply IP Table or firewall rules to block access
to dangerous
addresses. Finally, if none of the above protections are available, you could also run a
custom HTTP proxy
and force all requests through it to handle blocking dangerous addresses.
Example HTTP client that disallows access to loopback and RFC-1918 addresses
// IsDisallowedIP parses the ip to determine if we should allow the HTTP client to continue
func IsDisallowedIP(hostIP string) bool {
ip := net.ParseIP(hostIP)
return ip.IsMulticast() || ip.IsUnspecified() || ip.IsLoopback() || ip.IsPrivate()
}
// SafeTransport uses the net.Dial to connect, then if successful check if the resolved
// ip address is disallowed. We do this due to hosts such as localhost.lol being resolvable to
// potentially malicious URLs. We allow connection only for resolution purposes.
func SafeTransport(timeout time.Duration) *http.Transport {
return &http.Transport{
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
c, err := net.DialTimeout(network, addr, timeout)
if err != nil {
return nil, err
}
ip, _, _ := net.SplitHostPort(c.RemoteAddr().String())
if IsDisallowedIP(ip) {
return nil, errors.New("ip address is not allowed")
}
return c, err
},
DialTLS: func(network, addr string) (net.Conn, error) {
dialer := &net.Dialer{Timeout: timeout}
c, err := tls.DialWithDialer(dialer, network, addr, &tls.Config{})
if err != nil {
return nil, err
}
ip, _, _ := net.SplitHostPort(c.RemoteAddr().String())
if IsDisallowedIP(ip) {
return nil, errors.New("ip address is not allowed")
}
err = c.Handshake()
if err != nil {
return c, err
}
return c, c.Handshake()
},
TLSHandshakeTimeout: timeout,
}
}
func httpRequest(requestUrl string) {
const clientConnectTimeout = time.Second * 10
httpClient := &http.Client{
Transport: SafeTransport(clientConnectTimeout),
}
resp, err := httpClient.Get(requestUrl)
if err != nil {
log.Fatal(err)
}
defer resp.Body.Close()
// work with resp
}
For more information on SSRF see OWASP:
https://owasp.org/www-community/attacks/Server_Side_Request_Forgery
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>plugin/scripts/run-semgrep/main.go</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/semgrep/mcp-marketplace/blob/e66c0176b03b267cfeb16a188b0c804936e65dd9/plugin/scripts/run-semgrep/main.go#L211 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 211] os.Getenv("SEMGREP_FRAGMENT_URL")</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/semgrep/mcp-marketplace/blob/e66c0176b03b267cfeb16a188b0c804936e65dd9/plugin/scripts/run-semgrep/main.go#L211 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 211] url</a>"]
v3["<a href=https://github.com/semgrep/mcp-marketplace/blob/e66c0176b03b267cfeb16a188b0c804936e65dd9/plugin/scripts/run-semgrep/main.go#L283 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 283] requestScan</a>"]
v4["<a href=https://github.com/semgrep/mcp-marketplace/blob/e66c0176b03b267cfeb16a188b0c804936e65dd9/plugin/scripts/run-semgrep/main.go#L161 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 161] url</a>"]
end
v2 --> v3
v3 --> v4
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/semgrep/mcp-marketplace/blob/e66c0176b03b267cfeb16a188b0c804936e65dd9/plugin/scripts/run-semgrep/main.go#L170 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 170] http.NewRequest("POST", url, bytes.NewReader(body))</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by G107-1.
You can view more details about this finding in the Semgrep AppSec Platform.
| if token == "" { | ||
| return false | ||
| } | ||
| req, err := http.NewRequest("GET", semgrepURL()+"/api/agent/deployments/current", nil) |
There was a problem hiding this comment.
Semgrep identified an issue in your code:
Server-Side-Request-Forgery (SSRF) exploits backend systems that initiate requests to third
parties.
If user input is used in constructing or sending these requests, an attacker could supply
malicious
data to force the request to other systems or modify request data to cause unwanted actions.
Ensure user input is not used directly in constructing URLs or URIs when initiating requests
to third party
systems from back end systems. Care must also be taken when constructing payloads using user
input. Where
possible restrict to known URIs or payloads. Consider using a server side map where key's are
used to return
URLs such as https://site/goto?key=1 where {key: 1, url: 'http://some.url/', key: 2, url: 'http://...'}.
If you must use user supplied input for requesting URLs, it is strongly recommended that the
HTTP client
chosen allows you to customize and block certain IP ranges at the network level. By blocking
RFC 1918
addresses or other network address ranges, you can limit the severity of a successful SSRF
attack. Care must
also be taken to block certain protocol or address formatting such as IPv6.
If you can not block address ranges at the client level, you may want to run the HTTP client
as a protected
user, or in a protected network where you can apply IP Table or firewall rules to block access
to dangerous
addresses. Finally, if none of the above protections are available, you could also run a
custom HTTP proxy
and force all requests through it to handle blocking dangerous addresses.
Example HTTP client that disallows access to loopback and RFC-1918 addresses
// IsDisallowedIP parses the ip to determine if we should allow the HTTP client to continue
func IsDisallowedIP(hostIP string) bool {
ip := net.ParseIP(hostIP)
return ip.IsMulticast() || ip.IsUnspecified() || ip.IsLoopback() || ip.IsPrivate()
}
// SafeTransport uses the net.Dial to connect, then if successful check if the resolved
// ip address is disallowed. We do this due to hosts such as localhost.lol being resolvable to
// potentially malicious URLs. We allow connection only for resolution purposes.
func SafeTransport(timeout time.Duration) *http.Transport {
return &http.Transport{
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
c, err := net.DialTimeout(network, addr, timeout)
if err != nil {
return nil, err
}
ip, _, _ := net.SplitHostPort(c.RemoteAddr().String())
if IsDisallowedIP(ip) {
return nil, errors.New("ip address is not allowed")
}
return c, err
},
DialTLS: func(network, addr string) (net.Conn, error) {
dialer := &net.Dialer{Timeout: timeout}
c, err := tls.DialWithDialer(dialer, network, addr, &tls.Config{})
if err != nil {
return nil, err
}
ip, _, _ := net.SplitHostPort(c.RemoteAddr().String())
if IsDisallowedIP(ip) {
return nil, errors.New("ip address is not allowed")
}
err = c.Handshake()
if err != nil {
return c, err
}
return c, c.Handshake()
},
TLSHandshakeTimeout: timeout,
}
}
func httpRequest(requestUrl string) {
const clientConnectTimeout = time.Second * 10
httpClient := &http.Client{
Transport: SafeTransport(clientConnectTimeout),
}
resp, err := httpClient.Get(requestUrl)
if err != nil {
log.Fatal(err)
}
defer resp.Body.Close()
// work with resp
}
For more information on SSRF see OWASP:
https://owasp.org/www-community/attacks/Server_Side_Request_Forgery
Dataflow graph
flowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>plugin/scripts/login/main.go</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/semgrep/mcp-marketplace/blob/e66c0176b03b267cfeb16a188b0c804936e65dd9/plugin/scripts/login/main.go#L31 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 31] os.Getenv("SEMGREP_URL")</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/semgrep/mcp-marketplace/blob/e66c0176b03b267cfeb16a188b0c804936e65dd9/plugin/scripts/login/main.go#L31 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 31] u</a>"]
v3["<a href=https://github.com/semgrep/mcp-marketplace/blob/e66c0176b03b267cfeb16a188b0c804936e65dd9/plugin/scripts/login/main.go#L110 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 110] semgrepURL</a>"]
end
v2 --> v3
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/semgrep/mcp-marketplace/blob/e66c0176b03b267cfeb16a188b0c804936e65dd9/plugin/scripts/login/main.go#L110 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 110] http.NewRequest("GET", semgrepURL()+"/api/agent/deployments/current", nil)</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
To resolve this comment:
🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by G107-1.
You can view more details about this finding in the Semgrep AppSec Platform.
1 participant
I asked Claude to rewrite the python files in go for me. They seem to work but I haven't checked the code line by line.