Resolve vulnerability: Uncontrolled resource consumption (Slowloris)

MR created from vulnerability: Uncontrolled resource consumption (Slowloris)

AI GENERATED FIX

The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. Use this feature with caution. Before you run a pipeline or apply the code changes, carefully review and test them, to ensure that they solve the vulnerability.

The large language model that generated the suggested code changes was provided with the entire file that contains the vulnerable lines of code. It is not aware of any functionality outside of this context.

Please see our documentation for more information about this feature and leave feedback in this issue.

Description:

Go's net/http serve functions may be vulnerable to resource consumption attacks if timeouts are not properly configured prior to starting the HTTP server. An adversary may open up thousands of connections but never complete sending all data, or never terminate the connections. This may lead to the server no longer accepting new connections.

To protect against this style of resource consumption attack, timeouts should be set in the net/http server prior to calling the listen or serve functions. What this means is that the default http.ListenAndServe and http.Serve functions should not be used in a production setting as they are unable to have timeouts configured. Instead a custom http.Server object must be created with the timeouts configured.

Example setting timeouts on a net/http server:

// All values chosen below are dependent on application logic and
// should be tailored per use-case
srv := &http.Server{
  Addr: "localhost:8000",
  // ReadHeaderTimeout is the amount of time allowed to read
  // request headers. The connection's read deadline is reset
  // after reading the headers and the Handler can decide what
  // is considered too slow for the body. If ReadHeaderTimeout
  // is zero, the value of ReadTimeout is used. If both are
  // zero, there is no timeout.
  ReadHeaderTimeout: 15 * time.Second,

  // ReadTimeout is the maximum duration for reading the entire
  // request, including the body. A zero or negative value means
  // there will be no timeout.
  //
  // Because ReadTimeout does not let Handlers make per-request
  // decisions on each request body's acceptable deadline or
  // upload rate, most users will prefer to use
  // ReadHeaderTimeout. It is valid to use them both.
  ReadTimeout: 15 * time.Second,

  // WriteTimeout is the maximum duration before timing out
  // writes of the response. It is reset whenever a new
  // request's header is read. Like ReadTimeout, it does not
  // let Handlers make decisions on a per-request basis.
  // A zero or negative value means there will be no timeout.
  WriteTimeout: 10 * time.Second,

  // IdleTimeout is the maximum amount of time to wait for the
  // next request when keep-alives are enabled. If IdleTimeout
  // is zero, the value of ReadTimeout is used. If both are
  // zero, there is no timeout.
  IdleTimeout: 30 * time.Second,
}

// For per request timeouts applications can wrap all `http.HandlerFunc(...)` in
// `http.TimeoutHandler`` and specify a timeout, but note the TimeoutHandler does not
// start ticking until all headers have been read.

// Listen with our custom server with timeouts configured
if err := srv.ListenAndServe(); err != nil {
  log.Fatal(err)
}

For more information on the http.Server timeouts, see: https://pkg.go.dev/net/http#Server

For information on setting request based timeouts, see: https://pkg.go.dev/net/http#TimeoutHandler

For more information on the Slowloris attack see: https://en.wikipedia.org/wiki/Slowloris_(computer_security)

• Severity: medium
• Confidence: unknown
• Location: internal/git/gittest/http_server.go:41

Analysis:

The vulnerability report indicates a potential "Uncontrolled resource consumption (Slowloris)" issue, which is associated with CWE-400. This type of vulnerability can lead to Denial of Service (DoS) attacks by exhausting server resources.

The specific part of the code flagged as vulnerable is:

s := &http.Server{Handler: gitHTTPBackend}

This line creates a new HTTP server without specifying any timeout settings. The absence of timeouts can make the server vulnerable to Slowloris attacks, where an attacker sends HTTP requests very slowly, keeping connections open for extended periods and potentially exhausting server resources.

However, it's important to note that this code is part of a test helper function (HTTPServer) used for setting up an HTTP server during testing. In a testing environment, the risk of a real-world Slowloris attack is minimal. Nevertheless, it's a good practice to set timeouts even in test code to mimic real-world scenarios and prevent potential issues in long-running tests.

Summary:

1. The reported vulnerability is an "Uncontrolled resource consumption (Slowloris)" issue, which could potentially lead to Denial of Service (DoS) attacks by exhausting server resources due to the lack of timeout settings in the HTTP server configuration.

2. The fix provided adds timeout settings to the HTTP server configuration:

s := &http.Server{
    Handler:      gitHTTPBackend,
    ReadTimeout:  10 * time.Second,
    WriteTimeout: 10 * time.Second,
    IdleTimeout:  120 * time.Second,
}

This addresses the security concern by:

• Setting a ReadTimeout of 10 seconds, which limits the time for reading the entire request, including the body.
• Setting a WriteTimeout of 10 seconds, which limits the time for writing the response to the client.
• Setting an IdleTimeout of 120 seconds, which limits the maximum amount of time to wait for the next request when keep-alives are enabled.

These timeout settings help mitigate Slowloris-type attacks by ensuring that connections are not held open indefinitely, thus preventing resource exhaustion.

While this fix is implemented in a test helper function, it's a good practice to include these settings even in test code. This ensures that the test environment more closely mimics a production environment and can help catch potential timeout-related issues during testing.

Identifiers:

• CWE-400
• Gosec Rule ID G112
• A6:2017 - Security Misconfiguration
• Gosec Rule ID gosec.G114-1
• Gosec Rule ID G114
• A05:2021 - Security Misconfiguration