Merge pull request #54 from cerberauth/cookies-unsecure-practices

feat: scan for insecure cookies practices

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.

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()
 }

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
+}

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)
+}