diff --git a/pkg/builder/builder.go b/pkg/builder/builder.go index b2281012..77cd4763 100644 --- a/pkg/builder/builder.go +++ b/pkg/builder/builder.go @@ -121,6 +121,12 @@ func (builder *Builder) Handler(w http.ResponseWriter, r *http.Request) { } logger.Info("builder received request", zap.Any("request", req)) + if !utils.ValidateFilePathComponent(req.SrcPkgFilename) { + e := "invalid source package filename" + logger.Error(e, zap.String("filename", req.SrcPkgFilename)) + builder.reply(r.Context(), w, "", e, http.StatusBadRequest) + return + } logger.Debug("starting build") srcPkgPath := filepath.Join(builder.sharedVolumePath, req.SrcPkgFilename) deployPkgFilename := fmt.Sprintf("%s-%s", req.SrcPkgFilename, strings.ToLower(uniuri.NewLen(6))) diff --git a/pkg/executor/executortype/poolmgr/gp.go b/pkg/executor/executortype/poolmgr/gp.go index 523f615b..4bed549b 100644 --- a/pkg/executor/executortype/poolmgr/gp.go +++ b/pkg/executor/executortype/poolmgr/gp.go @@ -304,16 +304,18 @@ func (gp *GenericPool) choosePod(ctx context.Context, newLabels map[string]strin // Relabel. If the pod already got picked and // modified, this should fail; in that case just // retry. - labelPatch, _ := json.Marshal(newLabels) - // Append executor instance id to pod annotations to // indicate this pod is managed by this executor. annotations := gp.getDeployAnnotations(gp.env) - annotationPatch, _ := json.Marshal(annotations) - - patch := fmt.Sprintf(`{"metadata":{"annotations":%v, "labels":%v}}`, string(annotationPatch), string(labelPatch)) - logger.Info("relabel pod", zap.String("pod", patch)) - newPod, err := gp.kubernetesClient.CoreV1().Pods(chosenPod.Namespace).Patch(ctx, chosenPod.Name, k8sTypes.StrategicMergePatchType, []byte(patch), metav1.PatchOptions{}) + patch := map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": annotations, + "labels": newLabels, + }, + } + patchBytes, _ := json.Marshal(patch) + logger.Info("relabel pod", zap.String("pod", string((patchBytes)))) + newPod, err := gp.kubernetesClient.CoreV1().Pods(chosenPod.Namespace).Patch(ctx, chosenPod.Name, k8sTypes.StrategicMergePatchType, patchBytes, metav1.PatchOptions{}) if err != nil && errors.Is(err, context.Canceled) { // ending retry loop when the request canceled gp.readyPodQueue.Done(key) diff --git a/pkg/fetcher/fetcher.go b/pkg/fetcher/fetcher.go index 6a417f86..667f1a6c 100644 --- a/pkg/fetcher/fetcher.go +++ b/pkg/fetcher/fetcher.go @@ -253,9 +253,9 @@ func (fetcher *Fetcher) SpecializeHandler(w http.ResponseWriter, r *http.Request func (fetcher *Fetcher) Fetch(ctx context.Context, pkg *fv1.Package, req FunctionFetchRequest) (int, error) { logger := otelUtils.LoggerWithTraceID(ctx, fetcher.logger) - // check that the requested filename is not an empty string and error out if so - if len(req.Filename) == 0 { - e := "fetch request received for an empty file name" + // check that the requested filename is not an empty string and doest not contain any path traversal + if !utils.ValidateFilePathComponent(req.Filename) { + e := "fetch request received for an invalid file name" logger.Error(e, zap.Any("request", req)) return http.StatusBadRequest, errors.New(fmt.Sprintf("%s, request: %v", e, req)) } @@ -406,6 +406,11 @@ func (fetcher *Fetcher) FetchSecretsAndCfgMaps(ctx context.Context, secrets []fv return httpCode, errors.New(e) } + if !utils.ValidateFilePathComponent(secret.Namespace) && !utils.ValidateFilePathComponent(secret.Name) { + e := "fetch request received for an invalid secret name or namespace" + logger.Error(e, zap.Any("request", secret)) + return http.StatusBadRequest, errors.New(fmt.Sprintf("%s, request: %v", e, secret)) + } secretPath := filepath.Join(secret.Namespace, secret.Name) secretDir := filepath.Join(fetcher.sharedSecretPath, secretPath) err = os.MkdirAll(secretDir, os.ModeDir|0750) @@ -454,6 +459,12 @@ func (fetcher *Fetcher) FetchSecretsAndCfgMaps(ctx context.Context, secrets []fv return httpCode, errors.New(e) } + if !utils.ValidateFilePathComponent(config.Namespace) && !utils.ValidateFilePathComponent(config.Name) { + e := "fetch request received for an invalid configmap name or namespace" + logger.Error(e, zap.Any("request", config)) + return http.StatusBadRequest, errors.New(fmt.Sprintf("%s, request: %v", e, config)) + } + configPath := filepath.Join(config.Namespace, config.Name) configDir := filepath.Join(fetcher.sharedConfigPath, configPath) err = os.MkdirAll(configDir, os.ModeDir|0750) @@ -519,6 +530,13 @@ func (fetcher *Fetcher) UploadHandler(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusBadRequest) return } + + if !utils.ValidateFilePathComponent(req.Filename) { + logger.Error("invalid filename in request", zap.String("filename", req.Filename)) + http.Error(w, "Invalid file name", http.StatusBadRequest) + return + } + logger.Info("fetcher received upload request", zap.Any("request", req)) zipFilename := req.Filename + ".zip" diff --git a/pkg/utils/serviceaccount.go b/pkg/utils/serviceaccount.go index 9e95cff7..d98bb1dc 100644 --- a/pkg/utils/serviceaccount.go +++ b/pkg/utils/serviceaccount.go @@ -96,7 +96,7 @@ func CreateMissingPermissionForSA(ctx context.Context, kubernetesClient kubernet if enableSA { interval := getSAInterval() logger.Debug("interval value", zap.Any("interval", interval)) - sa := getSAObj(ctx, kubernetesClient, logger) + sa := getSAObj(kubernetesClient, logger) logger.Info("Starting service account check", zap.Any("interval", interval)) if interval > 0 { go wait.UntilWithContext(ctx, sa.runSACheck, interval) @@ -106,7 +106,7 @@ func CreateMissingPermissionForSA(ctx context.Context, kubernetesClient kubernet } } -func getSAObj(ctx context.Context, kubernetesClient kubernetes.Interface, logger *zap.Logger) *ServiceAccount { +func getSAObj(kubernetesClient kubernetes.Interface, logger *zap.Logger) *ServiceAccount { saObj := &ServiceAccount{ kubernetesClient: kubernetesClient, logger: logger, diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index dbf57fd2..ca1a1704 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -180,6 +180,10 @@ func isHttp2xxSuccessful(status int) bool { } func DownloadUrl(ctx context.Context, httpClient *http.Client, url string, localPath string) error { + // validate local path for directory traversal attacks + if filepath.Clean(localPath) != localPath { + return errors.Errorf("invalid local path: %s", localPath) + } resp, err := ctxhttp.Get(ctx, httpClient, url) if err != nil { return err @@ -305,3 +309,18 @@ func IsOwnerReferencesEnabled() bool { disableOwnerReference, _ := strconv.ParseBool(os.Getenv(ENV_DISABLE_OWNER_REFERENCES)) return !disableOwnerReference } + +// ValidateFilePathComponent checks if the filename is valid to prevent directory traversal attacks. +func ValidateFilePathComponent(filename string) bool { + return len(filename) > 0 && !containsInvalidChars(filename) +} + +func containsInvalidChars(filename string) bool { + invalidChars := []string{"/", "\\", ".."} + for _, char := range invalidChars { + if strings.Contains(filename, char) { + return true + } + } + return false +}