From fe5e5f592b253930bf7c4994e2c0a2d6298f8aed Mon Sep 17 00:00:00 2001 From: Ankit Chawla Date: Mon, 21 Feb 2022 12:44:27 +0530 Subject: [PATCH] Added function to sanitize strings by escaping quotes (#2360) --- pkg/controller/configmapApi.go | 4 ++++ pkg/controller/secretApi.go | 4 ++++ pkg/router/functionHandler.go | 2 ++ pkg/storagesvc/storagesvc.go | 6 +++++- pkg/storagesvc/stowClient.go | 3 +++ pkg/utils/utils.go | 6 ++++++ pkg/utils/utils_test.go | 37 ++++++++++++++++++++++++++++++++++ 7 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pkg/controller/configmapApi.go b/pkg/controller/configmapApi.go index af653fba..e108a342 100644 --- a/pkg/controller/configmapApi.go +++ b/pkg/controller/configmapApi.go @@ -22,6 +22,8 @@ import ( "github.com/gorilla/mux" "go.uber.org/zap" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/fission/fission/pkg/utils" ) func (a *API) ConfigMapExists(w http.ResponseWriter, r *http.Request) { @@ -34,6 +36,8 @@ func (a *API) ConfigMapExists(w http.ResponseWriter, r *http.Request) { _, err := a.kubernetesClient.CoreV1().ConfigMaps(ns).Get(r.Context(), name, metav1.GetOptions{}) if err != nil { + name = utils.EscapeQuotes(name) + ns = utils.EscapeQuotes(ns) a.logger.Error("error getting config map", zap.Error(err), zap.String("config_map_name", name), zap.String("namespace", ns)) a.respondWithError(w, err) return diff --git a/pkg/controller/secretApi.go b/pkg/controller/secretApi.go index 5c0b2a58..91460b62 100644 --- a/pkg/controller/secretApi.go +++ b/pkg/controller/secretApi.go @@ -22,6 +22,8 @@ import ( "github.com/gorilla/mux" "go.uber.org/zap" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/fission/fission/pkg/utils" ) func (a *API) SecretExists(w http.ResponseWriter, r *http.Request) { @@ -34,6 +36,8 @@ func (a *API) SecretExists(w http.ResponseWriter, r *http.Request) { _, err := a.kubernetesClient.CoreV1().Secrets(ns).Get(r.Context(), name, metav1.GetOptions{}) if err != nil { + name = utils.EscapeQuotes(name) + ns = utils.EscapeQuotes(ns) a.logger.Error("error getting secret", zap.Error(err), zap.String("secret_name", name), diff --git a/pkg/router/functionHandler.go b/pkg/router/functionHandler.go index b6f6340a..f690a283 100644 --- a/pkg/router/functionHandler.go +++ b/pkg/router/functionHandler.go @@ -295,6 +295,7 @@ func (roundTripper *RetryingRoundTripper) RoundTrip(req *http.Request) (*http.Re req.URL.Path = "/" } + req.URL.Path = utils.EscapeQuotes(req.URL.Path) logger.Debug("function invoke url", zap.String("prefixTrim", prefixTrim), zap.Bool("keepPrefix", keepPrefix), @@ -386,6 +387,7 @@ func (roundTripper *RetryingRoundTripper) RoundTrip(req *http.Request) (*http.Re // Check whether an error is an timeout error ("dial tcp i/o timeout"). if isNetTimeoutErr { + req.URL.Host = utils.EscapeQuotes(req.URL.Host) logger.Debug("request errored out - backing off before retrying", zap.String("url", req.URL.Host), zap.Error(err)) diff --git a/pkg/storagesvc/storagesvc.go b/pkg/storagesvc/storagesvc.go index 8fadd452..35752e8a 100644 --- a/pkg/storagesvc/storagesvc.go +++ b/pkg/storagesvc/storagesvc.go @@ -31,6 +31,7 @@ import ( "go.opencensus.io/plugin/ochttp" "go.uber.org/zap" + "github.com/fission/fission/pkg/utils" "github.com/fission/fission/pkg/utils/otel" ) @@ -79,6 +80,9 @@ func (ss *StorageService) uploadHandler(w http.ResponseWriter, r *http.Request) } defer file.Close() + //sanitize string to prevent security issues + handler.Filename = utils.EscapeQuotes(handler.Filename) + // stow wants the file size, but that's different from the // content length, the content length being the size of the // encoded file in the HTTP request. So we require an @@ -96,7 +100,6 @@ func (ss *StorageService) uploadHandler(w http.ResponseWriter, r *http.Request) if err != nil { ss.logger.Error("error parsing 'X-File-Size' header", zap.Error(err), - zap.Strings("header", fileSizeS), zap.String("filename", handler.Filename)) http.Error(w, "missing or bad X-File-Size header", http.StatusBadRequest) return @@ -175,6 +178,7 @@ func (ss *StorageService) downloadHandler(w http.ResponseWriter, r *http.Request // stream it to response err = ss.storageClient.copyFileToStream(fileId, w) if err != nil { + fileId = utils.EscapeQuotes(fileId) ss.logger.Error("error getting file from storage client", zap.Error(err), zap.String("file_id", fileId)) if err == ErrNotFound { http.Error(w, "Error retrieving item: not found", http.StatusNotFound) diff --git a/pkg/storagesvc/stowClient.go b/pkg/storagesvc/stowClient.go index f1e43308..39113a71 100644 --- a/pkg/storagesvc/stowClient.go +++ b/pkg/storagesvc/stowClient.go @@ -27,6 +27,8 @@ import ( "github.com/graymeta/stow" "github.com/pkg/errors" "go.uber.org/zap" + + "github.com/fission/fission/pkg/utils" ) type ( @@ -173,6 +175,7 @@ func (client *StowClient) copyFileToStream(fileId string, w io.Writer) error { return ErrWritingFileIntoResponse } + fileId = utils.EscapeQuotes(fileId) client.logger.Debug("successfully wrote file into httpresponse", zap.String("file", fileId)) return nil } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index f3f1f1ea..b5551129 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -205,3 +205,9 @@ func DownloadUrl(ctx context.Context, httpClient *http.Client, url string, local return nil } + +func EscapeQuotes(str string) string { + replacer := strings.NewReplacer("\n", "", "\r", "", "\t", "", `"`, `\"`) + str = replacer.Replace(str) + return str +} diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 125a3558..f337f72e 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -81,3 +81,40 @@ func TestGetChecksum(t *testing.T) { }) } } + +func TestEscapeQuotes(t *testing.T) { + tests := []struct { + name string + src string + want string + }{ + { + name: "Testing tab escape sequence", + src: "This\tis\ta\ttest\t string.", + want: "Thisisatest string.", + }, + { + name: "Testing carriage return", + src: "This is a \rtest string. \r This is the second test string\r.", + want: "This is a test string. This is the second test string.", + }, + { + name: "Testing next line escape sequence", + src: "This is a \ntest string. \n This is the second test string\n.", + want: "This is a test string. This is the second test string.", + }, + { + name: "Testing quotes", + src: `This is a "test string"". This is" the second test string "."`, + want: `This is a \"test string\"\". This is\" the second test string \".\"`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := EscapeQuotes(tt.src) + if got != tt.want { + t.Errorf("EscapeQuotes() got = %v, want = %v", got, tt.want) + } + }) + } +}