From b056515c51dcd043fc33b512eab31cc6d631e325 Mon Sep 17 00:00:00 2001 From: Dan Kortschak Date: Wed, 10 Jul 2024 06:50:35 +0930 Subject: [PATCH] x-pack/filebeat/input/{cel,salesforce}: check response for nilness before logging (#40144) It turns out that retryablehttp will pass a nil *http.Response to the client's ErrorHandler function although this is not documented. In the cases where we are using this, this will result in a nil deference panic when the retries exceed their maximum. So, check for nilness to avoid this. --- CHANGELOG.next.asciidoc | 1 + x-pack/filebeat/input/cel/input.go | 13 +++++++++++-- x-pack/filebeat/input/salesforce/input.go | 13 +++++++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 6eb0e61151e..e2c7cea7d50 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -149,6 +149,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff] - Upgrade github.com/hashicorp/go-retryablehttp to mitigate CVE-2024-6104 {pull}40036[40036] - Fix for Google Workspace duplicate events issue by adding canonical sorting over fingerprint keys array to maintain key order. {pull}40055[40055] {issue}39859[39859] - Fix handling of deeply nested numeric values in HTTP Endpoint CEL programs. {pull}40115[40115] +- Prevent panic in CEL and salesforce inputs when github.com/hashicorp/go-retryablehttp exceeds maximum retries. {pull}40144[40144] *Heartbeat* diff --git a/x-pack/filebeat/input/cel/input.go b/x-pack/filebeat/input/cel/input.go index 9320491344e..d9e3f977909 100644 --- a/x-pack/filebeat/input/cel/input.go +++ b/x-pack/filebeat/input/cel/input.go @@ -913,10 +913,19 @@ func checkRedirect(cfg *ResourceConfig, log *logp.Logger) func(*http.Request, [] // retryErrorHandler returns a retryablehttp.ErrorHandler that will log retry resignation // but return the last retry attempt's response and a nil error so that the CEL code // can evaluate the response status itself. Any error passed to the retryablehttp.ErrorHandler -// is returned unaltered. +// is returned unaltered. Despite not being documented so, the error handler may be passed +// a nil resp. retryErrorHandler will handle this case. func retryErrorHandler(max int, log *logp.Logger) retryablehttp.ErrorHandler { return func(resp *http.Response, err error, numTries int) (*http.Response, error) { - log.Warnw("giving up retries", "method", resp.Request.Method, "url", resp.Request.URL, "retries", max+1) + if resp != nil && resp.Request != nil { + reqURL := "unavailable" + if resp.Request.URL != nil { + reqURL = resp.Request.URL.String() + } + log.Warnw("giving up retries", "method", resp.Request.Method, "url", reqURL, "retries", max+1) + } else { + log.Warnw("giving up retries: no response available", "retries", max+1) + } return resp, err } } diff --git a/x-pack/filebeat/input/salesforce/input.go b/x-pack/filebeat/input/salesforce/input.go index a54f66cc668..2029429ace0 100644 --- a/x-pack/filebeat/input/salesforce/input.go +++ b/x-pack/filebeat/input/salesforce/input.go @@ -470,10 +470,19 @@ func (l *retryLog) Warn(msg string, kv ...interface{}) { l.log.Warnw(msg, kv... // retryErrorHandler returns a retryablehttp.ErrorHandler that will log retry resignation // but return the last retry attempt's response and a nil error to allow the retryablehttp.Client // evaluate the response status itself. Any error passed to the retryablehttp.ErrorHandler -// is returned unaltered. +// is returned unaltered. Despite not being documented so, the error handler may be passed +// a nil resp. retryErrorHandler will handle this case. func retryErrorHandler(max int, log *logp.Logger) retryablehttp.ErrorHandler { return func(resp *http.Response, err error, numTries int) (*http.Response, error) { - log.Warnw("giving up retries", "method", resp.Request.Method, "url", resp.Request.URL, "retries", max+1) + if resp != nil && resp.Request != nil { + reqURL := "unavailable" + if resp.Request.URL != nil { + reqURL = resp.Request.URL.String() + } + log.Warnw("giving up retries", "method", resp.Request.Method, "url", reqURL, "retries", max+1) + } else { + log.Warnw("giving up retries: no response available", "retries", max+1) + } return resp, err } }