fix: Securing load_config path parsing#467
Conversation
| auto resolve_path = [&](const std::string& path) -> std::string { | ||
| std::filesystem::path fs_path = path; | ||
| // Canonicalize the path to resolve symbolic links and remove '..' or '.' | ||
| try { |
There was a problem hiding this comment.
It seems u r allowing the relative path here not rejecting it
There was a problem hiding this comment.
-
The check for std::filesystem::path(filename).is_absolute() ensures that only absolute paths are accepted. Any relative paths (including those like ../..) are immediately rejected.
-
The use of std::filesystem::canonical() ensures that the path is properly resolved (eliminating any . or .. references), ensuring that it points to the correct file. Ex. path = "../npu.json" resolves to "absolute/path/to/npu.json" here if its a valid location else you throw a runtime exception while resolving path.
There was a problem hiding this comment.
But it should not be allowed. What if it is pointing to a file with no access to current user ?
There was a problem hiding this comment.
@vthaniel can you please test this scenario,
|
Open Questions on Relative File Path access: To be tracked with a JIRA later |
2 participants
Description
This PR restricts user to only allow usage of absolute paths while using load_config feature in provider options.
Example usage:
Allowed:
load_config|/home/ubuntu/npu_cfg.jsonNot Allowed:
load_config|../../npu_cfg.json