From ba11394d0f6b9cced52ed89bb2ca056e37c39950 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Tue, 17 Mar 2026 22:48:42 +0100 Subject: [PATCH] Potential fix for code scanning alert no. 80: Uncontrolled data used in path expression Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- pkg/service/datastore/datastore.go | 54 ++++++++++++++++++++++++--- pkg/service/datastore/mapping_test.go | 2 +- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index c95e079..7650629 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -24,7 +24,11 @@ func exists(path string) bool { // DataStore represents the device and configuration storage. type DataStore struct { - DataDir string + // DataDir is the (possibly relative) base directory for all datastore files. + DataDir string + // baseDir is the absolute, normalized base directory used for path safety checks. + baseDir string + eventMutex sync.RWMutex deviceEvents map[string][]models.DeviceEvent idMutex sync.RWMutex @@ -54,28 +58,68 @@ func NewDataStore(dataDir string) *DataStore { dataDir = "data" } + absBase, err := filepath.Abs(dataDir) + if err != nil { + // Fallback to the provided dataDir if Abs fails; this preserves existing behavior. + absBase = dataDir + } + return &DataStore{ DataDir: dataDir, + baseDir: absBase, deviceEvents: make(map[string][]models.DeviceEvent), deviceMappings: make(map[string]string), } } +// safeJoin joins the given path elements to the datastore baseDir and ensures +// that the resulting absolute path stays within baseDir. If the check fails, +// baseDir is returned to prevent directory traversal. +func (ds *DataStore) safeJoin(elem ...string) string { + // Join the base directory with the provided elements. + path := filepath.Join(append([]string{ds.baseDir}, elem...)...) + + absPath, err := filepath.Abs(path) + if err != nil { + // On error, fall back to baseDir to avoid using an unexpected path. + return ds.baseDir + } + + base := ds.baseDir + if base == "" { + // If baseDir is not set for some reason, fall back to original path. + return absPath + } + + // Ensure the resolved path is within the base directory. + baseWithSep := base + if !strings.HasSuffix(baseWithSep, string(os.PathSeparator)) { + baseWithSep += string(os.PathSeparator) + } + + if absPath == base || strings.HasPrefix(absPath, baseWithSep) { + return absPath + } + + // If the path would escape the base directory, return baseDir as a safe default. + return base +} + // AccountDir returns the directory path for a specific account. func (ds *DataStore) AccountDir(account string) string { - return filepath.Join(ds.DataDir, "accounts", account) + return ds.safeJoin("accounts", account) } // AccountDevicesDir returns the devices directory path for a specific account. func (ds *DataStore) AccountDevicesDir(account string) string { - return filepath.Join(ds.AccountDir(account), constants.DevicesDir) + return ds.safeJoin("accounts", account, constants.DevicesDir) } // AccountDeviceDir returns the directory path for a specific device within an account. func (ds *DataStore) AccountDeviceDir(account, device string) string { // First, check if the device directory exists directly with the given deviceID // This prioritizes MAC-based deviceIDs over legacy mappings - directPath := filepath.Join(ds.AccountDevicesDir(account), device) + directPath := ds.safeJoin("accounts", account, constants.DevicesDir, device) if _, err := os.Stat(directPath); err == nil { // Directory exists, use the direct deviceID (preferred for MAC-based IDs) return directPath @@ -95,7 +139,7 @@ func (ds *DataStore) AccountDeviceDir(account, device string) string { if ok { // Use the mapped device only if it exists and the direct path doesn't - mappedPath := filepath.Join(ds.AccountDevicesDir(account), mappedDevice) + mappedPath := ds.safeJoin("accounts", account, constants.DevicesDir, mappedDevice) if _, err := os.Stat(mappedPath); err == nil { return mappedPath } diff --git a/pkg/service/datastore/mapping_test.go b/pkg/service/datastore/mapping_test.go index a92f0f0..f1600ab 100644 --- a/pkg/service/datastore/mapping_test.go +++ b/pkg/service/datastore/mapping_test.go @@ -57,7 +57,7 @@ func TestDataStore_MacAddressMapping(t *testing.T) { // Test mapping resolution in AccountDeviceDir resolvedDir := ds.AccountDeviceDir(accountID, macAddress) - expectedDir := filepath.Join("testdata/mapping", "accounts", accountID, "devices", serialNumber) + expectedDir, _ := filepath.Abs(filepath.Join("testdata/mapping", "accounts", accountID, "devices", serialNumber)) if resolvedDir != expectedDir { t.Errorf("expected dir %s, got %s", expectedDir, resolvedDir) }