Refactor file paths in JamfPro and MS Graph API request files to use filepath.Base() for secure file handling

Author: ShocOne

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

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

helpers/helpers.go

@@ -2,10 +2,34 @@
 package helpers
 
 import (
+	"fmt"
+	"os"
+	"path/filepath"
 	"time"
 )
 
 // ParseISO8601Date attempts to parse a string date in ISO 8601 format.
 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)
+}