Merge pull request #232 from cerberauth/fix-broken-authentication-fal…

…se-positive Fix authentication bypass false positive
cerberauth · Dec 7, 2024 · 5f0543c · 5f0543c
2 parents fcdbf3d + 1c86e4d
commit 5f0543c
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 1 deletion.
diff --git a/internal/request/client.go b/internal/request/client.go
@@ -5,6 +5,7 @@ import (
 	"net/url"
 	"time"
 
+	"github.com/cerberauth/vulnapi/internal/auth"
 	"go.uber.org/ratelimit"
 )
 
@@ -88,3 +89,37 @@ func (c *Client) WithCookies(cookies []*http.Cookie) *Client {
 	c.Cookies = cookies
 	return c
 }
+
+func removeCookie(cookies []*http.Cookie, cookie *http.Cookie) []*http.Cookie {
+	for i, c := range cookies {
+		if c == cookie {
+			return append(cookies[:i], cookies[i+1:]...)
+		}
+	}
+	return cookies
+}
+
+func (c *Client) ClearSecurityScheme(securityScheme *auth.SecurityScheme) *Client {
+	// delete security schemes headers and cookies when name and value are the same
+	for k, v := range securityScheme.GetHeaders() {
+		if c.Header.Get(k) == v[0] {
+			c.Header.Del(k)
+		}
+	}
+
+	for _, sc := range securityScheme.GetCookies() {
+		for _, cookie := range c.Cookies {
+			if cookie.Name == sc.Name && cookie.Value == sc.Value {
+				c.Cookies = removeCookie(c.Cookies, cookie)
+			}
+		}
+	}
+	return c
+}
+
+func (c *Client) ClearSecuritySchemes(securitySchemes []*auth.SecurityScheme) *Client {
+	for _, securityScheme := range securitySchemes {
+		c.ClearSecurityScheme(securityScheme)
+	}
+	return c
+}
diff --git a/internal/request/client_test.go b/internal/request/client_test.go
@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/cerberauth/vulnapi/internal/auth"
 	"github.com/cerberauth/vulnapi/internal/request"
 	"github.com/stretchr/testify/assert"
 )
@@ -62,3 +63,42 @@ func TestClient_WithCookies(t *testing.T) {
 
 	assert.Equal(t, cookies, client.Cookies)
 }
+
+func TestClient_ClearHeaderWithSecurityScheme(t *testing.T) {
+	client := request.NewClient(request.NewClientOptions{})
+	client.Header.Set("Authorization", "Bearer token")
+
+	value := "token"
+	securityScheme := auth.MustNewAuthorizationBearerSecurityScheme("token", &value)
+	client.ClearSecurityScheme(securityScheme)
+
+	assert.Empty(t, client.Header.Get("Authorization"))
+}
+
+// func TestClient_ClearCookieWithSecurityScheme(t *testing.T) {
+// 	client := request.NewClient(request.NewClientOptions{})
+// 	client.Cookies = []*http.Cookie{{Name: "session", Value: "12345"}}
+
+// 	value := "token"
+// 	securityScheme := &auth.SecurityScheme{
+// 		Cookies: []*http.Cookie{{Name: "session", Value: "12345"}},
+// 	}
+// 	client.ClearSecurityScheme(securityScheme)
+
+// 	assert.Empty(t, client.Header.Get("Authorization"))
+// }
+
+func TestClient_ClearSecuritySchemes(t *testing.T) {
+	client := request.NewClient(request.NewClientOptions{})
+	client.Header.Set("Authorization", "Bearer token")
+	client.Cookies = []*http.Cookie{{Name: "session", Value: "12345"}}
+
+	value := "token"
+	securityScheme1 := auth.MustNewAuthorizationBearerSecurityScheme("token", &value)
+	securityScheme2 := auth.MustNewNoAuthSecurityScheme()
+
+	client.ClearSecuritySchemes([]*auth.SecurityScheme{securityScheme1, securityScheme2})
+
+	assert.Empty(t, client.Header.Get("Authorization"))
+	// assert.Empty(t, client.Cookies)
+}
diff --git a/scenario/graphql.go b/scenario/graphql.go
@@ -27,6 +27,7 @@ func NewGraphQLScan(url string, client *request.Client, opts *scan.ScanOptions)
 	} else {
 		securitySchemes = []*auth.SecurityScheme{auth.MustNewNoAuthSecurityScheme()}
 	}
+	client.ClearSecuritySchemes(securitySchemes)
 
 	url = addDefaultProtocolWhenMissing(url)
 	op, err := operation.NewOperation(http.MethodPost, url, nil, client)

diff --git a/scenario/graphql_test.go b/scenario/graphql_test.go
@@ -68,6 +68,8 @@ func TestNewGraphQLScanWithUpperCaseAuthorizationHeader(t *testing.T) {
 	assert.Equal(t, server.URL, s.Operations[0].URL.String())
 	assert.Equal(t, http.MethodPost, s.Operations[0].Method)
 	assert.Equal(t, []*auth.SecurityScheme{auth.MustNewAuthorizationBearerSecurityScheme("default", &token)}, s.Operations[0].SecuritySchemes)
+	// Should clear client header after setting security schemes
+	assert.Empty(t, client.Header.Get("Authorization"))
 }
 
 func TestNewGraphQLScanWithUpperCaseAuthorizationAndLowerCaseBearerHeader(t *testing.T) {

diff --git a/scenario/openapi.go b/scenario/openapi.go
@@ -16,6 +16,9 @@ func NewOpenAPIScan(openapi *openapi.OpenAPI, securitySchemesValues *openapi.Sec
 	if err != nil {
 		return nil, err
 	}
+	for _, securityScheme := range securitySchemes {
+		client.ClearSecurityScheme(securityScheme)
+	}
 
 	operations, err := openapi.Operations(client, securitySchemes)
 	if err != nil {

diff --git a/scenario/url.go b/scenario/url.go
@@ -28,9 +28,9 @@ func NewURLScan(method string, url string, data string, client *request.Client,
 	} else {
 		securitySchemes = []*auth.SecurityScheme{auth.MustNewNoAuthSecurityScheme()}
 	}
+	client.ClearSecuritySchemes(securitySchemes)
 
 	body := bytes.NewBuffer([]byte(data))
-
 	url = addDefaultProtocolWhenMissing(url)
 	op, err := operation.NewOperation(method, url, body, client)
 	op.GenerateID()

diff --git a/scenario/url_test.go b/scenario/url_test.go
@@ -45,6 +45,8 @@ func TestNewURLScanWithUpperCaseAuthorizationHeader(t *testing.T) {
 	assert.Equal(t, server.URL, s.Operations[0].URL.String())
 	assert.Equal(t, http.MethodGet, s.Operations[0].Method)
 	assert.Equal(t, []*auth.SecurityScheme{auth.MustNewAuthorizationBearerSecurityScheme("default", &token)}, s.Operations[0].SecuritySchemes)
+	// Should clear client header after setting security schemes
+	assert.Empty(t, client.Header.Get("Authorization"))
 }
 
 func TestNewURLScanWithUpperCaseAuthorizationAndLowerCaseBearerHeader(t *testing.T) {