[#226] Improve CORS validation

Signed-off-by: Marina Biryukova <m.biryukova@yadro.com>

Makefile

@@ -31,10 +31,6 @@ PKG_VERSION ?= $(shell echo $(VERSION) | sed "s/^v//" | \
 .PHONY: debpackage debclean
 
 FUZZING_DIR = $(shell pwd)/tests/fuzzing/files
-FUZZING_TEMP_DIRS = $(shell find . -type f -name '*_fuzz_test.go' -exec dirname {} \; | uniq | xargs -I{} echo -n "{}/tempfuzz ")
-FUZZING_COVER_FILES = $(shell find . -type f -name '*_fuzz_test.go' -exec dirname {} \; | uniq | xargs -I{} echo -n "{}/cover.out ")
-FUZZING_FUNC_FILES = $(shell find . -type f -name '*_fuzz_test.go' -exec dirname {} \; | uniq | xargs -I{} echo -n "{}/func.txt ")
-FUZZING_INDEX_FILES = $(shell find . -type f -name '*_fuzz_test.go' -exec dirname {} \; | uniq | xargs -I{} echo -n "{}/index.html ")
 NGFUZZ_REPO = https://gitflic.ru/project/yadro/ngfuzz.git
 FUZZ_TIMEOUT ?= 30
 FUZZ_FUNCTIONS ?= ""
@@ -192,10 +188,7 @@ version:
 # Clean up
 clean:
 	rm -rf vendor
-	rm -rf $(BINDIR)
-	rm -rf $(FUZZING_DIR) $(FUZZING_TEMP_DIRS)
-	rm -f $(FUZZING_COVER_FILES) $(FUZZING_FUNC_FILES) $(FUZZING_INDEX_FILES)
-	git checkout -- go.mod go.sum
+	rm -rf $(BINDIR) 
 
 # Package for Debian
 debpackage:

internal/handler/cors.go

@@ -79,6 +79,10 @@ func (h *Handler) Preflight(req *fasthttp.RequestCtx) {
 	}
 
 	for _, rule := range corsConfig.CORSRules {
+		if err = checkCORSRuleWildcards(rule); err != nil {
+			log.Error(logs.InvalidCorsRule, zap.Error(err), logs.TagField(logs.TagDatapath))
+			continue
+		}
 		for _, o := range rule.AllowedOrigins {
 			if o == string(origin) || o == wildcard || (strings.Contains(o, "*") && match(o, string(origin))) {
 				for _, m := range rule.AllowedMethods {
@@ -147,6 +151,10 @@ func (h *Handler) SetCORSHeaders(req *fasthttp.RequestCtx) {
 	}
 
 	for _, rule := range corsConfig.CORSRules {
+		if err = checkCORSRuleWildcards(rule); err != nil {
+			log.Error(logs.InvalidCorsRule, zap.Error(err), logs.TagField(logs.TagDatapath))
+			continue
+		}
 		for _, o := range rule.AllowedOrigins {
 			if o == string(origin) || (strings.Contains(o, "*") && len(o) > 1 && match(o, string(origin))) {
 				for _, m := range rule.AllowedMethods {
@@ -178,6 +186,22 @@ func (h *Handler) SetCORSHeaders(req *fasthttp.RequestCtx) {
 	}
 }
 
+func checkCORSRuleWildcards(rule data.CORSRule) error {
+	for _, origin := range rule.AllowedOrigins {
+		if strings.Count(origin, wildcard) > 1 {
+			return fmt.Errorf("invalid allowed origin: %s", origin)
+		}
+	}
+
+	for _, header := range rule.AllowedHeaders {
+		if strings.Count(header, wildcard) > 1 {
+			return fmt.Errorf("invalid allowed header: %s", header)
+		}
+	}
+
+	return nil
+}
+
 func (h *Handler) getCORSConfig(ctx context.Context, log *zap.Logger, cidStr string) (*data.CORSConfiguration, error) {
 	cnrID, err := h.resolveContainer(ctx, cidStr)
 	if err != nil {

internal/handler/cors_test.go

@@ -176,6 +176,55 @@ func TestPreflight(t *testing.T) {
 				},
 				status: fasthttp.StatusBadRequest,
 			},
+			{
+				name: "invalid allowed origin",
+				corsConfig: &data.CORSConfiguration{
+					CORSRules: []data.CORSRule{
+						{
+							AllowedOrigins: []string{"*.example.*"},
+							AllowedMethods: []string{"HEAD"},
+						},
+					},
+				},
+				requestHeaders: map[string]string{
+					fasthttp.HeaderOrigin:                     "http://www.example.com",
+					fasthttp.HeaderAccessControlRequestMethod: "HEAD",
+				},
+				expectedHeaders: map[string]string{
+					fasthttp.HeaderAccessControlAllowOrigin:      "",
+					fasthttp.HeaderAccessControlAllowMethods:     "",
+					fasthttp.HeaderAccessControlAllowHeaders:     "",
+					fasthttp.HeaderAccessControlExposeHeaders:    "",
+					fasthttp.HeaderAccessControlMaxAge:           "",
+					fasthttp.HeaderAccessControlAllowCredentials: "",
+				},
+				status: fasthttp.StatusForbidden,
+			},
+			{
+				name: "invalid allowed header",
+				corsConfig: &data.CORSConfiguration{
+					CORSRules: []data.CORSRule{
+						{
+							AllowedOrigins: []string{"*example.com"},
+							AllowedMethods: []string{"HEAD"},
+							AllowedHeaders: []string{"x-amz-*-*"},
+						},
+					},
+				},
+				requestHeaders: map[string]string{
+					fasthttp.HeaderOrigin:                     "http://www.example.com",
+					fasthttp.HeaderAccessControlRequestMethod: "HEAD",
+				},
+				expectedHeaders: map[string]string{
+					fasthttp.HeaderAccessControlAllowOrigin:      "",
+					fasthttp.HeaderAccessControlAllowMethods:     "",
+					fasthttp.HeaderAccessControlAllowHeaders:     "",
+					fasthttp.HeaderAccessControlExposeHeaders:    "",
+					fasthttp.HeaderAccessControlMaxAge:           "",
+					fasthttp.HeaderAccessControlAllowCredentials: "",
+				},
+				status: fasthttp.StatusForbidden,
+			},
 		} {
 			t.Run(tc.name, func(t *testing.T) {
 				if tc.corsConfig != nil {
@@ -328,6 +377,47 @@ func TestSetCORSHeaders(t *testing.T) {
 					fasthttp.HeaderAccessControlAllowCredentials: "",
 				},
 			},
+			{
+				name: "invalid allowed origin",
+				corsConfig: &data.CORSConfiguration{
+					CORSRules: []data.CORSRule{
+						{
+							AllowedOrigins: []string{"*.example.*"},
+							AllowedMethods: []string{"GET"},
+						},
+					},
+				},
+				requestHeaders: map[string]string{
+					fasthttp.HeaderOrigin: "http://www.example.com",
+				},
+				expectedHeaders: map[string]string{
+					fasthttp.HeaderAccessControlAllowOrigin:      "",
+					fasthttp.HeaderAccessControlAllowMethods:     "",
+					fasthttp.HeaderVary:                          "",
+					fasthttp.HeaderAccessControlAllowCredentials: "",
+				},
+			},
+			{
+				name: "invalid allowed header",
+				corsConfig: &data.CORSConfiguration{
+					CORSRules: []data.CORSRule{
+						{
+							AllowedOrigins: []string{"*example.com"},
+							AllowedMethods: []string{"GET"},
+							AllowedHeaders: []string{"x-amz-*-*"},
+						},
+					},
+				},
+				requestHeaders: map[string]string{
+					fasthttp.HeaderOrigin: "http://www.example.com",
+				},
+				expectedHeaders: map[string]string{
+					fasthttp.HeaderAccessControlAllowOrigin:      "",
+					fasthttp.HeaderAccessControlAllowMethods:     "",
+					fasthttp.HeaderVary:                          "",
+					fasthttp.HeaderAccessControlAllowCredentials: "",
+				},
+			},
 		} {
 			t.Run(tc.name, func(t *testing.T) {
 				epoch++

internal/handler/handler_fuzz_test.go

@@ -24,6 +24,11 @@ import (
 	"go.uber.org/zap"
 )
 
+const (
+	fuzzSuccessExitCode = 0
+	fuzzFailExitCode    = -1
+)
+
 func prepareStrings(tp *go_fuzz_utils.TypeProvider, count int) ([]string, error) {
 	array := make([]string, count)
 	var err error
@@ -217,18 +222,23 @@ func InitFuzzUpload() {
 
 }
 
-func DoFuzzUpload(input []byte) {
+func DoFuzzUpload(input []byte) int {
 	// FUZZER INIT
 	if len(input) < 100 {
-		return
+		return fuzzFailExitCode
 	}
 
 	tp, err := go_fuzz_utils.NewTypeProvider(input)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
-	_, _, _, _, _, _, _, _ = upload(tp)
+	_, _, _, _, _, _, _, err = upload(tp)
+	if err != nil {
+		return fuzzFailExitCode
+	}
+
+	return fuzzSuccessExitCode
 }
 
 func FuzzUpload(f *testing.F) {
@@ -297,28 +307,30 @@ func InitFuzzGet() {
 
 }
 
-func DoFuzzGet(input []byte) {
+func DoFuzzGet(input []byte) int {
 	// FUZZER INIT
 	if len(input) < 100 {
-		return
+		return fuzzFailExitCode
 	}
 
 	tp, err := go_fuzz_utils.NewTypeProvider(input)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	ctx, hc, cnrID, resp, filename, _, _, err := upload(tp)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	r, err := downloadOrHead(tp, ctx, hc, cnrID, resp, filename)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	hc.Handler().DownloadByAddressOrBucketName(r)
+
+	return fuzzSuccessExitCode
 }
 
 func FuzzGet(f *testing.F) {
@@ -331,28 +343,30 @@ func InitFuzzHead() {
 
 }
 
-func DoFuzzHead(input []byte) {
+func DoFuzzHead(input []byte) int {
 	// FUZZER INIT
 	if len(input) < 100 {
-		return
+		return fuzzFailExitCode
 	}
 
 	tp, err := go_fuzz_utils.NewTypeProvider(input)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	ctx, hc, cnrID, resp, filename, _, _, err := upload(tp)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	r, err := downloadOrHead(tp, ctx, hc, cnrID, resp, filename)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	hc.Handler().HeadByAddressOrBucketName(r)
+
+	return fuzzSuccessExitCode
 }
 
 func FuzzHead(f *testing.F) {
@@ -365,36 +379,36 @@ func InitFuzzDownloadByAttribute() {
 
 }
 
-func DoFuzzDownloadByAttribute(input []byte) {
+func DoFuzzDownloadByAttribute(input []byte) int {
 	// FUZZER INIT
 	if len(input) < 100 {
-		return
+		return fuzzFailExitCode
 	}
 
 	tp, err := go_fuzz_utils.NewTypeProvider(input)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	ctx, hc, cnrID, _, _, attrKey, attrVal, err := upload(tp)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	cid := cnrID.EncodeToString()
 	cid, err = maybeFillRandom(tp, cid)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	attrKey, err = maybeFillRandom(tp, attrKey)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	attrVal, err = maybeFillRandom(tp, attrVal)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	r := new(fasthttp.RequestCtx)
@@ -404,6 +418,8 @@ func DoFuzzDownloadByAttribute(input []byte) {
 	r.SetUserValue("attr_val", attrVal)
 
 	hc.Handler().DownloadByAttribute(r)
+
+	return fuzzSuccessExitCode
 }
 
 func FuzzDownloadByAttribute(f *testing.F) {
@@ -416,36 +432,36 @@ func InitFuzzHeadByAttribute() {
 
 }
 
-func DoFuzzHeadByAttribute(input []byte) {
+func DoFuzzHeadByAttribute(input []byte) int {
 	// FUZZER INIT
 	if len(input) < 100 {
-		return
+		return fuzzFailExitCode
 	}
 
 	tp, err := go_fuzz_utils.NewTypeProvider(input)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	ctx, hc, cnrID, _, _, attrKey, attrVal, err := upload(tp)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	cid := cnrID.EncodeToString()
 	cid, err = maybeFillRandom(tp, cid)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	attrKey, err = maybeFillRandom(tp, attrKey)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	attrVal, err = maybeFillRandom(tp, attrVal)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	r := new(fasthttp.RequestCtx)
@@ -455,6 +471,8 @@ func DoFuzzHeadByAttribute(input []byte) {
 	r.SetUserValue("attr_val", attrVal)
 
 	hc.Handler().HeadByAttribute(r)
+
+	return fuzzSuccessExitCode
 }
 
 func FuzzHeadByAttribute(f *testing.F) {
@@ -467,32 +485,32 @@ func InitFuzzDownloadZipped() {
 
 }
 
-func DoFuzzDownloadZipped(input []byte) {
+func DoFuzzDownloadZipped(input []byte) int {
 	// FUZZER INIT
 	if len(input) < 100 {
-		return
+		return fuzzFailExitCode
 	}
 
 	tp, err := go_fuzz_utils.NewTypeProvider(input)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	ctx, hc, cnrID, _, _, _, _, err := upload(tp)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	cid := cnrID.EncodeToString()
 	cid, err = maybeFillRandom(tp, cid)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	prefix := ""
 	prefix, err = maybeFillRandom(tp, prefix)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	r := new(fasthttp.RequestCtx)
@@ -501,6 +519,8 @@ func DoFuzzDownloadZipped(input []byte) {
 	r.SetUserValue("prefix", prefix)
 
 	hc.Handler().DownloadZip(r)
+
+	return fuzzSuccessExitCode
 }
 
 func FuzzDownloadZipped(f *testing.F) {
@@ -513,21 +533,21 @@ func InitFuzzStoreBearerTokenAppCtx() {
 
 }
 
-func DoFuzzStoreBearerTokenAppCtx(input []byte) {
+func DoFuzzStoreBearerTokenAppCtx(input []byte) int {
 	// FUZZER INIT
 	if len(input) < 100 {
-		return
+		return fuzzFailExitCode
 	}
 
 	tp, err := go_fuzz_utils.NewTypeProvider(input)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	prefix := ""
 	prefix, err = maybeFillRandom(tp, prefix)
 	if err != nil {
-		return
+		return fuzzFailExitCode
 	}
 
 	ctx := context.Background()
@@ -550,6 +570,8 @@ func DoFuzzStoreBearerTokenAppCtx(input []byte) {
 	}
 
 	tokens.StoreBearerTokenAppCtx(ctx, r)
+
+	return fuzzSuccessExitCode
 }
 
 func FuzzStoreBearerTokenAppCtx(f *testing.F) {

internal/logs/logs.go

@@ -127,6 +127,7 @@ const (
 	EmptyAccessControlRequestMethodHeader = "empty Access-Control-Request-Method request header"
 	CORSRuleWasNotMatched                 = "cors rule was not matched"
 	CouldntCacheCors                      = "couldn't cache cors"
+	InvalidCorsRule                       = "invalid cors rule, skip"
 )
 
 // Log messages with the "external_storage" tag.