From 4f5bbd30770b87c7dc0af3a59fd5abe9f0a120e3 Mon Sep 17 00:00:00 2001 From: Emmanuel Gautier Date: Wed, 6 Mar 2024 18:18:01 +0100 Subject: [PATCH] feat: scan for insecure cookies practices --- README.md | 1 + scan/best_practices.go | 6 +- scan/best_practices/http_cookies.go | 80 +++++++++ scan/best_practices/http_cookies_test.go | 203 +++++++++++++++++++++++ 4 files changed, 289 insertions(+), 1 deletion(-) create mode 100644 scan/best_practices/http_cookies.go create mode 100644 scan/best_practices/http_cookies_test.go diff --git a/README.md b/README.md index 38a7f9a..8530ac2 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,7 @@ The scanner also detects the following security best practices: * X-Content-Type-Options Header is not set * X-Frame-Options Header is not set * HTTP Trace Method enabled +* HTTP Cookies not marked as secure, httpOnly, or SameSite * Server Signature exposed > More vulnerabilities and best practices will be added in future releases. If you have any suggestions or requests for additional vulnerabilities or best practices to be included, please feel free to open an issue or submit a pull request. diff --git a/scan/best_practices.go b/scan/best_practices.go index 164b3c9..54d69d3 100644 --- a/scan/best_practices.go +++ b/scan/best_practices.go @@ -16,6 +16,10 @@ func (s *Scan) WithServerSignatureScan() *Scan { return s.AddScanHandler(bestpractices.ServerSignatureScanHandler) } +func (s *Scan) WithHTTPCookiesBestPracticesScan() *Scan { + return s.AddScanHandler(bestpractices.HTTPCookiesScanHandler) +} + func (s *Scan) WithAllBestPracticesScans() *Scan { - return s.WithHTTPHeadersBestPracticesScan().WithHTTPTraceMethodBestPracticesScan().WithServerSignatureScan() + return s.WithHTTPHeadersBestPracticesScan().WithHTTPTraceMethodBestPracticesScan().WithServerSignatureScan().WithHTTPCookiesBestPracticesScan() } diff --git a/scan/best_practices/http_cookies.go b/scan/best_practices/http_cookies.go new file mode 100644 index 0000000..9562b0b --- /dev/null +++ b/scan/best_practices/http_cookies.go @@ -0,0 +1,80 @@ +package bestpractices + +import ( + "net/http" + + "github.com/cerberauth/vulnapi/internal/auth" + "github.com/cerberauth/vulnapi/internal/request" + "github.com/cerberauth/vulnapi/internal/scan" + "github.com/cerberauth/vulnapi/report" +) + +const ( + HTTPCookiesNotHTTPOnlySeverityLevel = 1 + HTTPCookiesNotHTTPOnlyVulnerabilityName = "Cookies not HTTP-Only" + HTTPCookiesNotHTTPOnlyVulnerabilityDescription = "Cookies should be http-only." + + HTTPCookiesNotSecureSeverityLevel = 1 + HTTPCookiesNotSecureVulnerabilityName = "Cookies not Secure" + HTTPCookiesNotSecureVulnerabilityDescription = "Cookies should be secure." + + HTTPCookiesSameSiteSeverityLevel = 1 + HTTPCookiesSameSiteVulnerabilityName = "Cookies SameSite not set or set to None" + HTTPCookiesSameSiteVulnerabilityDescription = "Cookies should have SameSite attribute set to Strict or Lax." + + HTTPCookiesExpiresSeverityLevel = 1 + HTTPCookiesExpiresVulnerabilityName = "Cookies Expires not set" + HTTPCookiesExpiresVulnerabilityDescription = "Cookies should have Expires attribute set." +) + +func HTTPCookiesScanHandler(operation *request.Operation, securityScheme auth.SecurityScheme) (*report.ScanReport, error) { + r := report.NewScanReport() + + securityScheme.SetAttackValue(securityScheme.GetValidValue()) + attempt, err := scan.ScanURL(operation, &securityScheme) + r.AddScanAttempt(attempt).End() + if err != nil { + return r, err + } + + // Detect every cookies insecure practices + for _, cookie := range attempt.Response.Cookies() { + if !cookie.Secure { + r.AddVulnerabilityReport(&report.VulnerabilityReport{ + SeverityLevel: HTTPCookiesNotSecureSeverityLevel, + Name: HTTPCookiesNotSecureVulnerabilityName, + Description: HTTPCookiesNotSecureVulnerabilityDescription, + Operation: operation, + }) + } + + if !cookie.HttpOnly { + r.AddVulnerabilityReport(&report.VulnerabilityReport{ + SeverityLevel: HTTPCookiesNotHTTPOnlySeverityLevel, + Name: HTTPCookiesNotHTTPOnlyVulnerabilityName, + Description: HTTPCookiesNotHTTPOnlyVulnerabilityDescription, + Operation: operation, + }) + } + + if cookie.SameSite == http.SameSiteNoneMode { + r.AddVulnerabilityReport(&report.VulnerabilityReport{ + SeverityLevel: HTTPCookiesSameSiteSeverityLevel, + Name: HTTPCookiesSameSiteVulnerabilityName, + Description: HTTPCookiesSameSiteVulnerabilityDescription, + Operation: operation, + }) + } + + if cookie.Expires.IsZero() { + r.AddVulnerabilityReport(&report.VulnerabilityReport{ + SeverityLevel: HTTPCookiesExpiresSeverityLevel, + Name: HTTPCookiesExpiresVulnerabilityName, + Description: HTTPCookiesExpiresVulnerabilityDescription, + Operation: operation, + }) + } + } + + return r, nil +} diff --git a/scan/best_practices/http_cookies_test.go b/scan/best_practices/http_cookies_test.go new file mode 100644 index 0000000..f23104c --- /dev/null +++ b/scan/best_practices/http_cookies_test.go @@ -0,0 +1,203 @@ +package bestpractices_test + +import ( + "net/http" + "testing" + "time" + + "github.com/cerberauth/vulnapi/internal/auth" + "github.com/cerberauth/vulnapi/internal/request" + "github.com/cerberauth/vulnapi/report" + bestpractices "github.com/cerberauth/vulnapi/scan/best_practices" + "github.com/jarcoal/httpmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHTTPCookiesScanHandlerWhenNoCookies(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + securityScheme := auth.NewNoAuthSecurityScheme() + operation := request.NewOperation("http://localhost:8080/", "GET", nil, nil, nil) + + httpmock.RegisterResponder(operation.Method, operation.Url, httpmock.NewBytesResponder(405, nil)) + + report, err := bestpractices.HTTPCookiesScanHandler(operation, securityScheme) + + require.NoError(t, err) + assert.Equal(t, 1, httpmock.GetTotalCallCount()) + assert.False(t, report.HasVulnerabilityReport()) +} + +func TestHTTPCookiesScanHandlerWhenNoUnsecrurePractices(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + securityScheme := auth.NewNoAuthSecurityScheme() + operation := request.NewOperation("http://localhost:8080/", "GET", nil, nil, nil) + + resp := httpmock.NewStringResponse(200, "OK") + cookie := &http.Cookie{ + Name: "cookie_name", + Value: "cookie_value", + Path: "/", + Domain: "localhost", + SameSite: http.SameSiteLaxMode, + Secure: true, + HttpOnly: true, + Expires: time.Now().Add(24 * time.Hour), + } + resp.Header.Add("Set-Cookie", cookie.String()) + httpmock.RegisterResponder(operation.Method, operation.Url, httpmock.ResponderFromResponse(resp)) + + report, err := bestpractices.HTTPCookiesScanHandler(operation, securityScheme) + + require.NoError(t, err) + assert.Equal(t, 1, httpmock.GetTotalCallCount()) + assert.False(t, report.HasVulnerabilityReport()) +} + +func TestHTTPCookiesScanHandlerWhenNotHttpOnly(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + securityScheme := auth.NewNoAuthSecurityScheme() + operation := request.NewOperation("http://localhost:8080/", "GET", nil, nil, nil) + + resp := httpmock.NewStringResponse(200, "OK") + cookie := &http.Cookie{ + Name: "cookie_name", + Value: "cookie_value", + Path: "/", + Domain: "localhost", + SameSite: http.SameSiteLaxMode, + Secure: true, + HttpOnly: false, + Expires: time.Now().Add(24 * time.Hour), + } + resp.Header.Add("Set-Cookie", cookie.String()) + httpmock.RegisterResponder(operation.Method, operation.Url, httpmock.ResponderFromResponse(resp)) + + expectedReport := report.VulnerabilityReport{ + SeverityLevel: bestpractices.HTTPCookiesNotHTTPOnlySeverityLevel, + Name: bestpractices.HTTPCookiesNotHTTPOnlyVulnerabilityName, + Description: bestpractices.HTTPCookiesNotHTTPOnlyVulnerabilityDescription, + Operation: operation, + } + + report, err := bestpractices.HTTPCookiesScanHandler(operation, securityScheme) + + require.NoError(t, err) + assert.Equal(t, 1, httpmock.GetTotalCallCount()) + assert.True(t, report.HasVulnerabilityReport()) + assert.Equal(t, report.GetVulnerabilityReports()[0], &expectedReport) +} + +func TestHTTPCookiesScanHandlerWhenNotSecure(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + securityScheme := auth.NewNoAuthSecurityScheme() + operation := request.NewOperation("http://localhost:8080/", "GET", nil, nil, nil) + + resp := httpmock.NewStringResponse(200, "OK") + cookie := &http.Cookie{ + Name: "cookie_name", + Value: "cookie_value", + Path: "/", + Domain: "localhost", + SameSite: http.SameSiteLaxMode, + Secure: false, + HttpOnly: true, + Expires: time.Now().Add(24 * time.Hour), + } + resp.Header.Add("Set-Cookie", cookie.String()) + httpmock.RegisterResponder(operation.Method, operation.Url, httpmock.ResponderFromResponse(resp)) + + expectedReport := report.VulnerabilityReport{ + SeverityLevel: bestpractices.HTTPCookiesNotSecureSeverityLevel, + Name: bestpractices.HTTPCookiesNotSecureVulnerabilityName, + Description: bestpractices.HTTPCookiesNotSecureVulnerabilityDescription, + Operation: operation, + } + + report, err := bestpractices.HTTPCookiesScanHandler(operation, securityScheme) + + require.NoError(t, err) + assert.Equal(t, 1, httpmock.GetTotalCallCount()) + assert.True(t, report.HasVulnerabilityReport()) + assert.Equal(t, report.GetVulnerabilityReports()[0], &expectedReport) +} + +func TestHTTPCookiesScanHandlerWhenSameSiteNone(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + securityScheme := auth.NewNoAuthSecurityScheme() + operation := request.NewOperation("http://localhost:8080/", "GET", nil, nil, nil) + + resp := httpmock.NewStringResponse(200, "OK") + cookie := &http.Cookie{ + Name: "cookie_name", + Value: "cookie_value", + Path: "/", + Domain: "localhost", + SameSite: http.SameSiteNoneMode, + Secure: true, + HttpOnly: true, + Expires: time.Now().Add(24 * time.Hour), + } + resp.Header.Add("Set-Cookie", cookie.String()) + httpmock.RegisterResponder(operation.Method, operation.Url, httpmock.ResponderFromResponse(resp)) + + expectedReport := report.VulnerabilityReport{ + SeverityLevel: bestpractices.HTTPCookiesSameSiteSeverityLevel, + Name: bestpractices.HTTPCookiesSameSiteVulnerabilityName, + Description: bestpractices.HTTPCookiesSameSiteVulnerabilityDescription, + Operation: operation, + } + + report, err := bestpractices.HTTPCookiesScanHandler(operation, securityScheme) + + require.NoError(t, err) + assert.Equal(t, 1, httpmock.GetTotalCallCount()) + assert.True(t, report.HasVulnerabilityReport()) + assert.Equal(t, report.GetVulnerabilityReports()[0], &expectedReport) +} + +func TestHTTPCookiesScanHandlerWhenExpiresNotSet(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + securityScheme := auth.NewNoAuthSecurityScheme() + operation := request.NewOperation("http://localhost:8080/", "GET", nil, nil, nil) + + resp := httpmock.NewStringResponse(200, "OK") + cookie := &http.Cookie{ + Name: "cookie_name", + Value: "cookie_value", + Path: "/", + Domain: "localhost", + SameSite: http.SameSiteLaxMode, + Secure: true, + HttpOnly: true, + Expires: time.Time{}, + } + resp.Header.Add("Set-Cookie", cookie.String()) + httpmock.RegisterResponder(operation.Method, operation.Url, httpmock.ResponderFromResponse(resp)) + + expectedReport := report.VulnerabilityReport{ + SeverityLevel: bestpractices.HTTPCookiesExpiresSeverityLevel, + Name: bestpractices.HTTPCookiesExpiresVulnerabilityName, + Description: bestpractices.HTTPCookiesExpiresVulnerabilityDescription, + Operation: operation, + } + + report, err := bestpractices.HTTPCookiesScanHandler(operation, securityScheme) + + require.NoError(t, err) + assert.Equal(t, 1, httpmock.GetTotalCallCount()) + assert.True(t, report.HasVulnerabilityReport()) + assert.Equal(t, report.GetVulnerabilityReports()[0], &expectedReport) +}