From 871ae5d76352d68eb9fe62a805d27ce1bc377d61 Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Wed, 30 Apr 2025 16:06:04 +0300 Subject: [PATCH 1/3] [#226] Improve CORS validation Signed-off-by: Marina Biryukova --- internal/handler/cors.go | 24 ++++++++++ internal/handler/cors_test.go | 90 +++++++++++++++++++++++++++++++++++ internal/logs/logs.go | 1 + 3 files changed, 115 insertions(+) diff --git a/internal/handler/cors.go b/internal/handler/cors.go index 7e8db93..a4b8b6b 100644 --- a/internal/handler/cors.go +++ b/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 { diff --git a/internal/handler/cors_test.go b/internal/handler/cors_test.go index 1ac07d7..629449f 100644 --- a/internal/handler/cors_test.go +++ b/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++ diff --git a/internal/logs/logs.go b/internal/logs/logs.go index 86921dd..68b8835 100644 --- a/internal/logs/logs.go +++ b/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. From 3aed60aa791f7a6381cfdfc7b752ffbcee77ac0a Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Tue, 6 May 2025 18:17:25 +0300 Subject: [PATCH 2/3] [#241] Remove exit codes from fuzzing tests Signed-off-by: Marina Biryukova --- internal/handler/handler_fuzz_test.go | 98 +++++++++++---------------- 1 file changed, 38 insertions(+), 60 deletions(-) diff --git a/internal/handler/handler_fuzz_test.go b/internal/handler/handler_fuzz_test.go index ff38b11..66d225b 100644 --- a/internal/handler/handler_fuzz_test.go +++ b/internal/handler/handler_fuzz_test.go @@ -24,11 +24,6 @@ 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 @@ -222,23 +217,18 @@ func InitFuzzUpload() { } -func DoFuzzUpload(input []byte) int { +func DoFuzzUpload(input []byte) { // FUZZER INIT if len(input) < 100 { - return fuzzFailExitCode + return } tp, err := go_fuzz_utils.NewTypeProvider(input) if err != nil { - return fuzzFailExitCode + return } - _, _, _, _, _, _, _, err = upload(tp) - if err != nil { - return fuzzFailExitCode - } - - return fuzzSuccessExitCode + _, _, _, _, _, _, _, _ = upload(tp) } func FuzzUpload(f *testing.F) { @@ -307,30 +297,28 @@ func InitFuzzGet() { } -func DoFuzzGet(input []byte) int { +func DoFuzzGet(input []byte) { // FUZZER INIT if len(input) < 100 { - return fuzzFailExitCode + return } tp, err := go_fuzz_utils.NewTypeProvider(input) if err != nil { - return fuzzFailExitCode + return } ctx, hc, cnrID, resp, filename, _, _, err := upload(tp) if err != nil { - return fuzzFailExitCode + return } r, err := downloadOrHead(tp, ctx, hc, cnrID, resp, filename) if err != nil { - return fuzzFailExitCode + return } hc.Handler().DownloadByAddressOrBucketName(r) - - return fuzzSuccessExitCode } func FuzzGet(f *testing.F) { @@ -343,30 +331,28 @@ func InitFuzzHead() { } -func DoFuzzHead(input []byte) int { +func DoFuzzHead(input []byte) { // FUZZER INIT if len(input) < 100 { - return fuzzFailExitCode + return } tp, err := go_fuzz_utils.NewTypeProvider(input) if err != nil { - return fuzzFailExitCode + return } ctx, hc, cnrID, resp, filename, _, _, err := upload(tp) if err != nil { - return fuzzFailExitCode + return } r, err := downloadOrHead(tp, ctx, hc, cnrID, resp, filename) if err != nil { - return fuzzFailExitCode + return } hc.Handler().HeadByAddressOrBucketName(r) - - return fuzzSuccessExitCode } func FuzzHead(f *testing.F) { @@ -379,36 +365,36 @@ func InitFuzzDownloadByAttribute() { } -func DoFuzzDownloadByAttribute(input []byte) int { +func DoFuzzDownloadByAttribute(input []byte) { // FUZZER INIT if len(input) < 100 { - return fuzzFailExitCode + return } tp, err := go_fuzz_utils.NewTypeProvider(input) if err != nil { - return fuzzFailExitCode + return } ctx, hc, cnrID, _, _, attrKey, attrVal, err := upload(tp) if err != nil { - return fuzzFailExitCode + return } cid := cnrID.EncodeToString() cid, err = maybeFillRandom(tp, cid) if err != nil { - return fuzzFailExitCode + return } attrKey, err = maybeFillRandom(tp, attrKey) if err != nil { - return fuzzFailExitCode + return } attrVal, err = maybeFillRandom(tp, attrVal) if err != nil { - return fuzzFailExitCode + return } r := new(fasthttp.RequestCtx) @@ -418,8 +404,6 @@ func DoFuzzDownloadByAttribute(input []byte) int { r.SetUserValue("attr_val", attrVal) hc.Handler().DownloadByAttribute(r) - - return fuzzSuccessExitCode } func FuzzDownloadByAttribute(f *testing.F) { @@ -432,36 +416,36 @@ func InitFuzzHeadByAttribute() { } -func DoFuzzHeadByAttribute(input []byte) int { +func DoFuzzHeadByAttribute(input []byte) { // FUZZER INIT if len(input) < 100 { - return fuzzFailExitCode + return } tp, err := go_fuzz_utils.NewTypeProvider(input) if err != nil { - return fuzzFailExitCode + return } ctx, hc, cnrID, _, _, attrKey, attrVal, err := upload(tp) if err != nil { - return fuzzFailExitCode + return } cid := cnrID.EncodeToString() cid, err = maybeFillRandom(tp, cid) if err != nil { - return fuzzFailExitCode + return } attrKey, err = maybeFillRandom(tp, attrKey) if err != nil { - return fuzzFailExitCode + return } attrVal, err = maybeFillRandom(tp, attrVal) if err != nil { - return fuzzFailExitCode + return } r := new(fasthttp.RequestCtx) @@ -471,8 +455,6 @@ func DoFuzzHeadByAttribute(input []byte) int { r.SetUserValue("attr_val", attrVal) hc.Handler().HeadByAttribute(r) - - return fuzzSuccessExitCode } func FuzzHeadByAttribute(f *testing.F) { @@ -485,32 +467,32 @@ func InitFuzzDownloadZipped() { } -func DoFuzzDownloadZipped(input []byte) int { +func DoFuzzDownloadZipped(input []byte) { // FUZZER INIT if len(input) < 100 { - return fuzzFailExitCode + return } tp, err := go_fuzz_utils.NewTypeProvider(input) if err != nil { - return fuzzFailExitCode + return } ctx, hc, cnrID, _, _, _, _, err := upload(tp) if err != nil { - return fuzzFailExitCode + return } cid := cnrID.EncodeToString() cid, err = maybeFillRandom(tp, cid) if err != nil { - return fuzzFailExitCode + return } prefix := "" prefix, err = maybeFillRandom(tp, prefix) if err != nil { - return fuzzFailExitCode + return } r := new(fasthttp.RequestCtx) @@ -519,8 +501,6 @@ func DoFuzzDownloadZipped(input []byte) int { r.SetUserValue("prefix", prefix) hc.Handler().DownloadZip(r) - - return fuzzSuccessExitCode } func FuzzDownloadZipped(f *testing.F) { @@ -533,21 +513,21 @@ func InitFuzzStoreBearerTokenAppCtx() { } -func DoFuzzStoreBearerTokenAppCtx(input []byte) int { +func DoFuzzStoreBearerTokenAppCtx(input []byte) { // FUZZER INIT if len(input) < 100 { - return fuzzFailExitCode + return } tp, err := go_fuzz_utils.NewTypeProvider(input) if err != nil { - return fuzzFailExitCode + return } prefix := "" prefix, err = maybeFillRandom(tp, prefix) if err != nil { - return fuzzFailExitCode + return } ctx := context.Background() @@ -570,8 +550,6 @@ func DoFuzzStoreBearerTokenAppCtx(input []byte) int { } tokens.StoreBearerTokenAppCtx(ctx, r) - - return fuzzSuccessExitCode } func FuzzStoreBearerTokenAppCtx(f *testing.F) { From 5e6ee3a41c72a84f1fd8494ab706408523d5a7c1 Mon Sep 17 00:00:00 2001 From: Marina Biryukova Date: Wed, 7 May 2025 10:29:54 +0300 Subject: [PATCH 3/3] [#241] Add fuzzing files removing to make clean Signed-off-by: Marina Biryukova --- Makefile | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 11084f0..6dd2222 100755 --- a/Makefile +++ b/Makefile @@ -31,6 +31,10 @@ 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 ?= "" @@ -188,7 +192,10 @@ version: # Clean up clean: rm -rf vendor - rm -rf $(BINDIR) + 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 # Package for Debian debpackage: