From c5c552ec84afef03cecaf3c1743621d7bfe921b3 Mon Sep 17 00:00:00 2001 From: ShocOne <62835948+ShocOne@users.noreply.github.com> Date: Thu, 18 Apr 2024 07:58:36 +0100 Subject: [PATCH] Refactor file paths in JamfPro and MS Graph API request files to use filepath.Base() for secure file handling --- .../jamfpro/jamfpro_api_request.go | 16 +++++++------ .../msgraph/msgraph_api_request.go | 17 +++++++------ helpers/helpers.go | 24 +++++++++++++++++++ 3 files changed, 43 insertions(+), 14 deletions(-) diff --git a/apiintegrations/jamfpro/jamfpro_api_request.go b/apiintegrations/jamfpro/jamfpro_api_request.go index a183d68..c740b71 100644 --- a/apiintegrations/jamfpro/jamfpro_api_request.go +++ b/apiintegrations/jamfpro/jamfpro_api_request.go @@ -7,9 +7,10 @@ import ( "encoding/xml" "io" "mime/multipart" - "os" + "path/filepath" "strings" + "github.com/deploymenttheory/go-api-http-client/helpers" "github.com/deploymenttheory/go-api-http-client/logger" "go.uber.org/zap" ) @@ -55,7 +56,7 @@ func (j *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoin return data, nil } -// MarshalMultipartFormData takes a map with form fields and file paths and returns the encoded body and content type. +// MarshalMultipartRequest handles multipart form data encoding with secure file handling and returns the encoded body and content type. func (j *JamfAPIHandler) MarshalMultipartRequest(fields map[string]string, files map[string]string, log logger.Logger) ([]byte, string, error) { body := &bytes.Buffer{} writer := multipart.NewWriter(body) @@ -67,15 +68,16 @@ func (j *JamfAPIHandler) MarshalMultipartRequest(fields map[string]string, files } } - // Add the files to the form data - for formField, filepath := range files { - file, err := os.Open(filepath) + // Add the files to the form data, using safeOpenFile to ensure secure file access + for formField, filePath := range files { + file, err := helpers.SafeOpenFile(filePath) if err != nil { + log.Error("Failed to open file securely", zap.String("file", filePath), zap.Error(err)) return nil, "", err } defer file.Close() - part, err := writer.CreateFormFile(formField, filepath) + part, err := writer.CreateFormFile(formField, filepath.Base(filePath)) if err != nil { return nil, "", err } @@ -84,7 +86,7 @@ func (j *JamfAPIHandler) MarshalMultipartRequest(fields map[string]string, files } } - // Close the writer before returning + // Close the writer to finish writing the multipart message contentType := writer.FormDataContentType() if err := writer.Close(); err != nil { return nil, "", err diff --git a/apiintegrations/msgraph/msgraph_api_request.go b/apiintegrations/msgraph/msgraph_api_request.go index 07dcf81..a066fea 100644 --- a/apiintegrations/msgraph/msgraph_api_request.go +++ b/apiintegrations/msgraph/msgraph_api_request.go @@ -6,8 +6,9 @@ import ( "encoding/json" "io" "mime/multipart" - "os" + "path/filepath" + "github.com/deploymenttheory/go-api-http-client/helpers" "github.com/deploymenttheory/go-api-http-client/logger" "go.uber.org/zap" ) @@ -29,8 +30,9 @@ func (g *GraphAPIHandler) MarshalRequest(body interface{}, method string, endpoi return data, nil } -// MarshalMultipartFormData takes a map with form fields and file paths and returns the encoded body and content type. +// MarshalMultipartRequest handles multipart form data encoding with secure file handling and returns the encoded body and content type. func (g *GraphAPIHandler) MarshalMultipartRequest(fields map[string]string, files map[string]string, log logger.Logger) ([]byte, string, error) { + body := &bytes.Buffer{} writer := multipart.NewWriter(body) @@ -41,15 +43,16 @@ func (g *GraphAPIHandler) MarshalMultipartRequest(fields map[string]string, file } } - // Add the files to the form data - for formField, filepath := range files { - file, err := os.Open(filepath) + // Add the files to the form data, using safeOpenFile to ensure secure file access + for formField, filePath := range files { + file, err := helpers.SafeOpenFile(filePath) if err != nil { + log.Error("Failed to open file securely", zap.String("file", filePath), zap.Error(err)) return nil, "", err } defer file.Close() - part, err := writer.CreateFormFile(formField, filepath) + part, err := writer.CreateFormFile(formField, filepath.Base(filePath)) if err != nil { return nil, "", err } @@ -58,7 +61,7 @@ func (g *GraphAPIHandler) MarshalMultipartRequest(fields map[string]string, file } } - // Close the writer before returning + // Close the writer to finish writing the multipart message contentType := writer.FormDataContentType() if err := writer.Close(); err != nil { return nil, "", err diff --git a/helpers/helpers.go b/helpers/helpers.go index 6887004..269ddce 100644 --- a/helpers/helpers.go +++ b/helpers/helpers.go @@ -2,6 +2,9 @@ package helpers import ( + "fmt" + "os" + "path/filepath" "time" ) @@ -9,3 +12,24 @@ import ( func ParseISO8601Date(dateStr string) (time.Time, error) { return time.Parse(time.RFC3339, dateStr) } + +// SafeOpenFile opens a file safely after validating and resolving its path. +func SafeOpenFile(filePath string) (*os.File, error) { + // Clean the file path to remove any ".." or similar components that can lead to directory traversal + cleanPath := filepath.Clean(filePath) + + // Resolve the clean path to an absolute path and ensure it resolves any symbolic links + absPath, err := filepath.EvalSymlinks(cleanPath) + if err != nil { + return nil, fmt.Errorf("unable to resolve the absolute path: %s, error: %w", filePath, err) + } + + // Optionally, check if the absolute path is within a permitted directory (omitted here for brevity) + // Example: allowedPathPrefix := "/safe/directory/" + // if !strings.HasPrefix(absPath, allowedPathPrefix) { + // return nil, fmt.Errorf("access to the file path is not allowed: %s", absPath) + // } + + // Open the file if the path is deemed safe + return os.Open(absPath) +}