mirror of
https://github.com/projectdiscovery/nuclei.git
synced 2026-01-31 15:53:10 +08:00
fix(http): resolve timeout config issues (#6562)
across multiple layers Fixes timeout configuration conflicts where HTTP requests would timeout prematurely despite configured values in `@timeout` annotations or `-timeout` flags. RCA: * `retryablehttp` pkg overriding with default 30s timeout. * Custom timeouts not propagating to `retryablehttp` layer. * Multiple timeout layers not sync properly. Changes: * Propagate custom timeouts from `@timeout` annotations to `retryablehttp` layer. * Adjust 5-minute maximum cap to prevent DoS via extremely large timeouts. * Ensure `retryableHttpOptions.Timeout` respects `ResponseHeaderTimeout`. * Add comprehensive tests for timeout capping behavior. This allows templates to override global timeout via `@timeout` annotations while preventing abuse thru unreasonably large timeout values. Fixes #6560. Signed-off-by: Dwi Siswanto <git@dw1.io>
This commit is contained in:
@@ -214,6 +214,10 @@ func wrappedGet(options *types.Options, configuration *Configuration) (*retryabl
|
||||
|
||||
retryableHttpOptions.RetryWaitMax = 10 * time.Second
|
||||
retryableHttpOptions.RetryMax = options.Retries
|
||||
retryableHttpOptions.Timeout = time.Duration(options.Timeout) * time.Second
|
||||
if configuration.ResponseHeaderTimeout > 0 && configuration.ResponseHeaderTimeout > retryableHttpOptions.Timeout {
|
||||
retryableHttpOptions.Timeout = configuration.ResponseHeaderTimeout
|
||||
}
|
||||
redirectFlow := configuration.RedirectFlow
|
||||
maxRedirects := configuration.MaxRedirects
|
||||
|
||||
@@ -261,6 +265,11 @@ func wrappedGet(options *types.Options, configuration *Configuration) (*retryabl
|
||||
if configuration.ResponseHeaderTimeout != 0 {
|
||||
responseHeaderTimeout = configuration.ResponseHeaderTimeout
|
||||
}
|
||||
|
||||
if responseHeaderTimeout < retryableHttpOptions.Timeout {
|
||||
responseHeaderTimeout = retryableHttpOptions.Timeout
|
||||
}
|
||||
|
||||
if configuration.Connection != nil && configuration.Connection.CustomMaxTimeout > 0 {
|
||||
responseHeaderTimeout = configuration.Connection.CustomMaxTimeout
|
||||
}
|
||||
|
||||
@@ -825,6 +825,7 @@ func (request *Request) executeRequest(input *contextargs.Context, generatedRequ
|
||||
connConfiguration := request.connConfiguration.Clone()
|
||||
modifiedConfig = connConfiguration
|
||||
}
|
||||
|
||||
modifiedConfig.ResponseHeaderTimeout = updatedTimeout.Timeout
|
||||
}
|
||||
|
||||
|
||||
@@ -29,7 +29,9 @@ var (
|
||||
reTimeoutAnnotation = regexp.MustCompile(`(?m)^@timeout:\s*(.+)\s*$`)
|
||||
// @once sets the request to be executed only once for a specific URL
|
||||
reOnceAnnotation = regexp.MustCompile(`(?m)^@once\s*$`)
|
||||
|
||||
// maxAnnotationTimeout is the maximum timeout allowed for @timeout
|
||||
// annotations to prevent DoS attacks via extremely large timeout values.
|
||||
maxAnnotationTimeout = 5 * time.Minute
|
||||
// ErrTimeoutAnnotationDeadline is the error returned when a specific amount of time was exceeded for a request
|
||||
// which was allotted using @timeout annotation this usually means that vulnerability was not found
|
||||
// in rare case it could also happen due to network congestion
|
||||
@@ -129,16 +131,17 @@ func (r *Request) parseAnnotations(rawRequest string, request *retryablehttp.Req
|
||||
|
||||
if duration := reTimeoutAnnotation.FindStringSubmatch(rawRequest); len(duration) > 0 {
|
||||
value := strings.TrimSpace(duration[1])
|
||||
if parsed, err := time.ParseDuration(value); err == nil {
|
||||
// to avoid dos via timeout request annotation in http template we set it to maximum of 2 minutes
|
||||
if parsed > 2*time.Minute {
|
||||
parsed = 2 * time.Minute
|
||||
if parsedTimeout, err := time.ParseDuration(value); err == nil {
|
||||
// Cap at maximum allowed timeout to prevent DoS via extremely large timeout values
|
||||
if parsedTimeout > maxAnnotationTimeout {
|
||||
parsedTimeout = maxAnnotationTimeout
|
||||
}
|
||||
|
||||
//nolint:govet // cancelled automatically by withTimeout
|
||||
// global timeout is overridden by annotation by replacing context
|
||||
ctx, overrides.cancelFunc = context.WithTimeoutCause(context.TODO(), parsed, ErrTimeoutAnnotationDeadline)
|
||||
ctx, overrides.cancelFunc = context.WithTimeoutCause(context.TODO(), parsedTimeout, ErrTimeoutAnnotationDeadline)
|
||||
// add timeout value to context
|
||||
ctx = context.WithValue(ctx, httpclientpool.WithCustomTimeout{}, httpclientpool.WithCustomTimeout{Timeout: parsed})
|
||||
ctx = context.WithValue(ctx, httpclientpool.WithCustomTimeout{}, httpclientpool.WithCustomTimeout{Timeout: parsedTimeout})
|
||||
request = request.Clone(ctx)
|
||||
}
|
||||
} else {
|
||||
|
||||
@@ -4,12 +4,26 @@ import (
|
||||
"context"
|
||||
"net/http"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/projectdiscovery/gologger"
|
||||
"github.com/projectdiscovery/nuclei/v3/pkg/protocols"
|
||||
"github.com/projectdiscovery/nuclei/v3/pkg/protocols/http/httpclientpool"
|
||||
"github.com/projectdiscovery/nuclei/v3/pkg/testutils"
|
||||
"github.com/projectdiscovery/retryablehttp-go"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func getExecuterOptions(t *testing.T) *protocols.ExecutorOptions {
|
||||
t.Helper()
|
||||
|
||||
options := testutils.DefaultOptions.Copy()
|
||||
options.Logger = &gologger.Logger{}
|
||||
testutils.Init(options)
|
||||
|
||||
return testutils.NewMockExecuterOptions(options, nil)
|
||||
}
|
||||
|
||||
func TestRequestParseAnnotationsSNI(t *testing.T) {
|
||||
t.Run("compliant-SNI-value", func(t *testing.T) {
|
||||
req := &Request{connConfiguration: &httpclientpool.Configuration{}}
|
||||
@@ -44,6 +58,7 @@ func TestRequestParseAnnotationsSNI(t *testing.T) {
|
||||
func TestRequestParseAnnotationsTimeout(t *testing.T) {
|
||||
t.Run("positive", func(t *testing.T) {
|
||||
request := &Request{
|
||||
options: getExecuterOptions(t),
|
||||
connConfiguration: &httpclientpool.Configuration{NoTimeout: true},
|
||||
}
|
||||
rawRequest := `@timeout: 2s
|
||||
@@ -56,12 +71,91 @@ func TestRequestParseAnnotationsTimeout(t *testing.T) {
|
||||
overrides, modified := request.parseAnnotations(rawRequest, httpReq)
|
||||
require.NotNil(t, overrides.cancelFunc, "could not initialize valid cancel function")
|
||||
require.True(t, modified, "could not get correct modified value")
|
||||
_, deadlined := overrides.request.Context().Deadline()
|
||||
|
||||
// Verify context has deadline
|
||||
deadline, deadlined := overrides.request.Context().Deadline()
|
||||
require.True(t, deadlined, "could not get set request deadline")
|
||||
|
||||
// Verify the timeout value is stored in context
|
||||
customTimeout, ok := overrides.request.Context().Value(httpclientpool.WithCustomTimeout{}).(httpclientpool.WithCustomTimeout)
|
||||
require.True(t, ok, "custom timeout not found in context")
|
||||
require.Equal(t, 2*time.Second, customTimeout.Timeout, "timeout value mismatch")
|
||||
|
||||
// Verify deadline is approximately 2 seconds from now
|
||||
expectedDeadline := time.Now().Add(2 * time.Second)
|
||||
require.WithinDuration(t, expectedDeadline, deadline, 100*time.Millisecond, "deadline not set correctly")
|
||||
})
|
||||
|
||||
t.Run("large-timeout", func(t *testing.T) {
|
||||
request := &Request{
|
||||
options: getExecuterOptions(t),
|
||||
connConfiguration: &httpclientpool.Configuration{NoTimeout: true},
|
||||
}
|
||||
|
||||
// Request a timeout of 10 minutes - should be capped at 5 minutes
|
||||
rawRequest := `@timeout: 10m
|
||||
GET / HTTP/1.1
|
||||
Host: {{Hostname}}`
|
||||
|
||||
httpReq, err := retryablehttp.NewRequest(http.MethodGet, "https://example.com", nil)
|
||||
require.Nil(t, err, "could not create http request")
|
||||
|
||||
overrides, modified := request.parseAnnotations(rawRequest, httpReq)
|
||||
require.NotNil(t, overrides.cancelFunc, "could not initialize valid cancel function")
|
||||
require.True(t, modified, "could not get correct modified value")
|
||||
|
||||
// Verify context has deadline
|
||||
deadline, deadlined := overrides.request.Context().Deadline()
|
||||
require.True(t, deadlined, "could not get set request deadline")
|
||||
|
||||
// Verify the timeout was capped at 5 minutes (not 10 minutes)
|
||||
customTimeout, ok := overrides.request.Context().Value(httpclientpool.WithCustomTimeout{}).(httpclientpool.WithCustomTimeout)
|
||||
require.True(t, ok, "custom timeout not found in context")
|
||||
|
||||
require.Equal(t, 5*time.Minute, customTimeout.Timeout, "timeout should be capped at 5 minutes")
|
||||
require.Less(t, customTimeout.Timeout, 10*time.Minute, "timeout should be less than requested 10 minutes")
|
||||
|
||||
// Verify deadline matches the capped timeout
|
||||
expectedDeadline := time.Now().Add(5 * time.Minute)
|
||||
require.WithinDuration(t, expectedDeadline, deadline, 100*time.Millisecond, "deadline not set to capped timeout")
|
||||
})
|
||||
|
||||
t.Run("below-cap-timeout", func(t *testing.T) {
|
||||
request := &Request{
|
||||
options: getExecuterOptions(t),
|
||||
connConfiguration: &httpclientpool.Configuration{NoTimeout: true},
|
||||
}
|
||||
|
||||
// Request a timeout of 2 minutes - should be allowed (below 5 minute cap)
|
||||
rawRequest := `@timeout: 2m
|
||||
GET / HTTP/1.1
|
||||
Host: {{Hostname}}`
|
||||
|
||||
httpReq, err := retryablehttp.NewRequest(http.MethodGet, "https://example.com", nil)
|
||||
require.Nil(t, err, "could not create http request")
|
||||
|
||||
overrides, modified := request.parseAnnotations(rawRequest, httpReq)
|
||||
require.NotNil(t, overrides.cancelFunc, "could not initialize valid cancel function")
|
||||
require.True(t, modified, "could not get correct modified value")
|
||||
|
||||
// Verify context has deadline
|
||||
deadline, deadlined := overrides.request.Context().Deadline()
|
||||
require.True(t, deadlined, "could not get set request deadline")
|
||||
|
||||
// Verify the timeout is NOT capped - should be 2 minutes
|
||||
customTimeout, ok := overrides.request.Context().Value(httpclientpool.WithCustomTimeout{}).(httpclientpool.WithCustomTimeout)
|
||||
require.True(t, ok, "custom timeout not found in context")
|
||||
|
||||
require.Equal(t, 2*time.Minute, customTimeout.Timeout, "timeout should be the requested 2 minutes")
|
||||
|
||||
// Verify deadline matches the requested timeout
|
||||
expectedDeadline := time.Now().Add(2 * time.Minute)
|
||||
require.WithinDuration(t, expectedDeadline, deadline, 100*time.Millisecond, "deadline not set to requested timeout")
|
||||
})
|
||||
|
||||
t.Run("negative", func(t *testing.T) {
|
||||
request := &Request{
|
||||
options: getExecuterOptions(t),
|
||||
connConfiguration: &httpclientpool.Configuration{},
|
||||
}
|
||||
rawRequest := `GET / HTTP/1.1
|
||||
|
||||
@@ -742,12 +742,12 @@ func (tv *Timeouts) ApplyDefaults() {
|
||||
if tv.TcpReadTimeout == 0 {
|
||||
tv.TcpReadTimeout = 5 * time.Second
|
||||
}
|
||||
if tv.HttpResponseHeaderTimeout == 0 {
|
||||
tv.HttpResponseHeaderTimeout = 10 * time.Second
|
||||
}
|
||||
if tv.HttpTimeout == 0 {
|
||||
tv.HttpTimeout = 3 * tv.DialTimeout
|
||||
}
|
||||
if tv.HttpResponseHeaderTimeout < tv.HttpTimeout {
|
||||
tv.HttpResponseHeaderTimeout = tv.HttpTimeout
|
||||
}
|
||||
if tv.JsCompilerExecutionTimeout == 0 {
|
||||
tv.JsCompilerExecutionTimeout = 2 * tv.DialTimeout
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user