diff --git a/pkg/protocols/http/httpclientpool/clientpool.go b/pkg/protocols/http/httpclientpool/clientpool.go index 4fa0790a5..14f4c8dc3 100644 --- a/pkg/protocols/http/httpclientpool/clientpool.go +++ b/pkg/protocols/http/httpclientpool/clientpool.go @@ -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 } diff --git a/pkg/protocols/http/request.go b/pkg/protocols/http/request.go index 47d795f9f..c538686e1 100644 --- a/pkg/protocols/http/request.go +++ b/pkg/protocols/http/request.go @@ -825,6 +825,7 @@ func (request *Request) executeRequest(input *contextargs.Context, generatedRequ connConfiguration := request.connConfiguration.Clone() modifiedConfig = connConfiguration } + modifiedConfig.ResponseHeaderTimeout = updatedTimeout.Timeout } diff --git a/pkg/protocols/http/request_annotations.go b/pkg/protocols/http/request_annotations.go index 217e21a34..c87e543c7 100644 --- a/pkg/protocols/http/request_annotations.go +++ b/pkg/protocols/http/request_annotations.go @@ -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 { diff --git a/pkg/protocols/http/request_annotations_test.go b/pkg/protocols/http/request_annotations_test.go index 778a0cb72..e8899d2f4 100644 --- a/pkg/protocols/http/request_annotations_test.go +++ b/pkg/protocols/http/request_annotations_test.go @@ -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 diff --git a/pkg/types/types.go b/pkg/types/types.go index 08ed924ff..daed01908 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -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 }