From 1a1b2ca51a94b9cf69d55812c17b44934873019a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=81h=CC=95=CC=B3m=CD=AD=CD=AD=CD=A8=CD=A9=CC=90e=CD=AC?= =?UTF-8?q?=CC=81=CD=8B=CD=AC=CC=8A=CC=93=CD=82=CC=98d?= <13666360+0x1@users.noreply.github.com> Date: Thu, 21 Sep 2023 11:20:40 -0400 Subject: [PATCH] updating uri detector to use tri-state verification (#1791) --- pkg/detectors/uri/uri.go | 32 ++++++++++++++++------------ pkg/detectors/uri/uri_test.go | 40 +++++++++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/pkg/detectors/uri/uri.go b/pkg/detectors/uri/uri.go index a09b20af4..8667ee617 100644 --- a/pkg/detectors/uri/uri.go +++ b/pkg/detectors/uri/uri.go @@ -15,6 +15,7 @@ import ( type Scanner struct { allowKnownTestSites bool + client *http.Client } // Ensure the Scanner satisfies the interface at compile time. @@ -23,7 +24,7 @@ var _ detectors.Detector = (*Scanner)(nil) var ( keyPat = regexp.MustCompile(`\b(?:https?:)?\/\/[\S]{3,50}:([\S]{3,50})@[-.%\w\/:]+\b`) - client = common.SaneHttpClient() + defaultClient = common.SaneHttpClient() ) // Keywords are used for efficiently pre-filtering chunks. @@ -71,7 +72,7 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result // Removing the path causes possible deduplication issues if some paths have basic auth and some do not. rawURL.Path = "" - s := detectors.Result{ + s1 := detectors.Result{ DetectorType: detectorspb.DetectorType_URI, Raw: []byte(rawURL.String()), RawV2: []byte(rawURLStr), @@ -79,27 +80,30 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result } if verify { - s.Verified = verifyURL(ctx, parsedURL) + if s.client == nil { + s.client = defaultClient + } + s1.Verified, s1.VerificationError = verifyURL(ctx, s.client, parsedURL) } - if !s.Verified { + if !s1.Verified { // Skip unverified findings where the password starts with a `$` - it's almost certainly a variable. if strings.HasPrefix(password, "$") { continue } } - if !s.Verified && detectors.IsKnownFalsePositive(string(s.Raw), detectors.DefaultFalsePositives, false) { + if !s1.Verified && !s.allowKnownTestSites && detectors.IsKnownFalsePositive(string(s1.Raw), detectors.DefaultFalsePositives, false) { continue } - results = append(results, s) + results = append(results, s1) } return results, nil } -func verifyURL(ctx context.Context, u *url.URL) bool { +func verifyURL(ctx context.Context, client *http.Client, u *url.URL) (bool, error) { // defuse most SSRF payloads u.Path = strings.TrimSuffix(u.Path, "/") u.RawQuery = "" @@ -112,41 +116,41 @@ func verifyURL(ctx context.Context, u *url.URL) bool { req, err := http.NewRequest("GET", credentialedURL, nil) if err != nil { - return false + return false, err } req = req.WithContext(ctx) credentialedRes, err := client.Do(req) if err != nil { - return false + return false, err } credentialedRes.Body.Close() // If the credentialed URL returns a non 2XX code, we can assume it's a false positive. if credentialedRes.StatusCode < 200 || credentialedRes.StatusCode > 299 { - return false + return false, nil } time.Sleep(time.Millisecond * 10) req, err = http.NewRequest("GET", nonCredentialedURL, nil) if err != nil { - return false + return false, err } req = req.WithContext(ctx) nonCredentialedRes, err := client.Do(req) if err != nil { - return false + return false, err } nonCredentialedRes.Body.Close() // If the non-credentialed URL returns a non 400-428 code and basic auth header, we can assume it's verified now. if nonCredentialedRes.StatusCode >= 400 && nonCredentialedRes.StatusCode < 429 { if nonCredentialedRes.Header.Get("WWW-Authenticate") != "" { - return true + return true, nil } } - return false + return false, nil } func (s Scanner) Type() detectorspb.DetectorType { diff --git a/pkg/detectors/uri/uri_test.go b/pkg/detectors/uri/uri_test.go index 260117c64..c1c1f840e 100644 --- a/pkg/detectors/uri/uri_test.go +++ b/pkg/detectors/uri/uri_test.go @@ -7,8 +7,10 @@ import ( "context" "fmt" "testing" + "time" "github.com/kylelemons/godebug/pretty" + "github.com/trufflesecurity/trufflehog/v3/pkg/common" "github.com/trufflesecurity/trufflehog/v3/pkg/detectors" "github.com/trufflesecurity/trufflehog/v3/pkg/pb/detectorspb" ) @@ -20,11 +22,12 @@ func TestURI_FromChunk(t *testing.T) { verify bool } tests := []struct { - name string - s Scanner - args args - want []detectors.Result - wantErr bool + name string + s Scanner + args args + want []detectors.Result + wantErr bool + wantVerificationErr bool }{ { name: "found, unverified, wrong username", @@ -77,6 +80,24 @@ func TestURI_FromChunk(t *testing.T) { }, wantErr: false, }, + { + name: "found, verified if not for timeout", + s: Scanner{client: common.SaneHttpClientTimeOut(1 * time.Microsecond)}, + args: args{ + ctx: context.Background(), + data: []byte(fmt.Sprintf("You can find a uri secret %s within", "https://httpwatch:pass@www.httpwatch.com/httpgallery/authentication/authenticatedimage/default.aspx")), + verify: true, + }, + want: []detectors.Result{ + { + DetectorType: detectorspb.DetectorType_URI, + Verified: false, + Redacted: "https://httpwatch:********@www.httpwatch.com", + }, + }, + wantErr: false, + wantVerificationErr: true, + }, { name: "bad scheme", s: Scanner{}, @@ -101,8 +122,8 @@ func TestURI_FromChunk(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := Scanner{allowKnownTestSites: true} - got, err := s.FromData(tt.args.ctx, tt.args.verify, tt.args.data) + tt.s.allowKnownTestSites = true + got, err := tt.s.FromData(tt.args.ctx, tt.args.verify, tt.args.data) if (err != nil) != tt.wantErr { t.Errorf("URI.FromData() error = %v, wantErr %v", err, tt.wantErr) return @@ -111,8 +132,13 @@ func TestURI_FromChunk(t *testing.T) { // return // } for i := range got { + if (got[i].VerificationError != nil) != tt.wantVerificationErr { + t.Errorf("URI.FromData() error = %v, wantVerificationErr %v", got[i].VerificationError, tt.wantErr) + return + } got[i].Raw = nil got[i].RawV2 = nil + got[i].VerificationError = nil } if diff := pretty.Compare(got, tt.want); diff != "" { t.Errorf("URI.FromData() %s diff: (-got +want)\n%s", tt.name, diff)