Description
LocalStore.getFilePath in pkg/state/local.go constructs a file path by joining basePath with a user-provided name via filepath.Join, but does not validate that the resolved path stays within basePath. A crafted name containing .. components (e.g. ../../../etc/passwd) can escape the designated state directory.
The #nosec G304 comments on every caller claim "filePath is controlled by getFilePath which ensures it's within our designated directory" — but getFilePath does not actually enforce this.
Impact
All LocalStore operations are affected: GetReader, GetWriter, CreateExclusive, Delete, and Exists. A malicious or accidentally crafted state name could read, write, or delete files outside the state directory.
Note: the workload-facing code paths already validate names via fileutils.ValidateWorkloadNameForPath, which rejects path separators and .. components. This issue affects the lower-level LocalStore directly, which is also used for RunConfig and group persistence where the validation may not be applied upstream.
Suggested fix
Add containment validation in getFilePath using the strings.HasPrefix(resolved, basePath + separator) pattern already established in pkg/fileutils/contained.go (WriteContainedFile).
I'd like to work on this and will submit a PR.
Description
LocalStore.getFilePathinpkg/state/local.goconstructs a file path by joiningbasePathwith a user-providednameviafilepath.Join, but does not validate that the resolved path stays withinbasePath. A crafted name containing..components (e.g.../../../etc/passwd) can escape the designated state directory.The
#nosec G304comments on every caller claim "filePath is controlled by getFilePath which ensures it's within our designated directory" — butgetFilePathdoes not actually enforce this.Impact
All
LocalStoreoperations are affected:GetReader,GetWriter,CreateExclusive,Delete, andExists. A malicious or accidentally crafted state name could read, write, or delete files outside the state directory.Note: the workload-facing code paths already validate names via
fileutils.ValidateWorkloadNameForPath, which rejects path separators and..components. This issue affects the lower-levelLocalStoredirectly, which is also used for RunConfig and group persistence where the validation may not be applied upstream.Suggested fix
Add containment validation in
getFilePathusing thestrings.HasPrefix(resolved, basePath + separator)pattern already established inpkg/fileutils/contained.go(WriteContainedFile).I'd like to work on this and will submit a PR.