Description
SharePointReader._download_file_by_url builds the local save path directly from the SharePoint item's name field, with no validation:
def _download_file_by_url(self, item: Dict[str, Any], download_dir: str) -> str:
file_name = item["name"]
content = self._get_file_content_by_url(item)
if not os.path.exists(download_dir):
os.makedirs(download_dir)
file_path = os.path.join(download_dir, file_name)
with open(file_path, "wb") as f:
f.write(content)
return file_path
(llama-index-integrations/readers/llama-index-readers-microsoft-sharepoint/llama_index/readers/microsoft_sharepoint/base.py, current main. Both _download_file and _download_files_from_sharepoint route through this same function, so it's the single write path for the reader.)
os.path.join doesn't sanitize its second argument — if file_name is an absolute path or contains ../, the write lands outside download_dir entirely:
>>> os.path.join('/tmp/sharepoint_downloads', '/etc/cron.d/evil')
'/etc/cron.d/evil'
>>> os.path.join('/tmp/sharepoint_downloads', '../../../../tmp/evil_via_traversal')
'/tmp/sharepoint_downloads/../../../../tmp/evil_via_traversal'
Ordinary SharePoint/OneDrive uploads through the web UI reject / and \ in file names, so this isn't trivially reachable through normal use. But the reader's own code has no defense of its own — it's relying entirely on an external service's client-side upload validation holding for every possible way an item can end up in a drive (Graph API calls from other tools, migrated/synced content from external systems, third-party connectors that write directly via the API), none of which are things this reader controls or can verify at read time. If any of those paths ever produce an item whose name contains a path separator, the write is unsandboxed.
This is the same class of issue as deepset-ai/haystack-core-integrations#3577 (the SharePoint/Google Drive fetchers there have the identical pattern), which is why I'm flagging it here too rather than assuming this reader is fine because the other one wasn't.
Suggested fix
Reject or strip path separators from file_name before joining, or resolve the final path and verify it's still contained within download_dir (Path(file_path).resolve().is_relative_to(Path(download_dir).resolve())), the same containment-check pattern already used elsewhere in this repo for path handling (e.g. the allowed_root check added to encode_image in llama-index-core's generic_utils.py).
Version
Confirmed present on current main.
Description
SharePointReader._download_file_by_urlbuilds the local save path directly from the SharePoint item'snamefield, with no validation:(
llama-index-integrations/readers/llama-index-readers-microsoft-sharepoint/llama_index/readers/microsoft_sharepoint/base.py, currentmain. Both_download_fileand_download_files_from_sharepointroute through this same function, so it's the single write path for the reader.)os.path.joindoesn't sanitize its second argument — iffile_nameis an absolute path or contains../, the write lands outsidedownload_direntirely:Ordinary SharePoint/OneDrive uploads through the web UI reject
/and\in file names, so this isn't trivially reachable through normal use. But the reader's own code has no defense of its own — it's relying entirely on an external service's client-side upload validation holding for every possible way an item can end up in a drive (Graph API calls from other tools, migrated/synced content from external systems, third-party connectors that write directly via the API), none of which are things this reader controls or can verify at read time. If any of those paths ever produce an item whosenamecontains a path separator, the write is unsandboxed.This is the same class of issue as
deepset-ai/haystack-core-integrations#3577(the SharePoint/Google Drive fetchers there have the identical pattern), which is why I'm flagging it here too rather than assuming this reader is fine because the other one wasn't.Suggested fix
Reject or strip path separators from
file_namebefore joining, or resolve the final path and verify it's still contained withindownload_dir(Path(file_path).resolve().is_relative_to(Path(download_dir).resolve())), the same containment-check pattern already used elsewhere in this repo for path handling (e.g. theallowed_rootcheck added toencode_imagein llama-index-core'sgeneric_utils.py).Version
Confirmed present on current
main.