security: validate alias paths in list_bento to prevent path traversal#1219
Open
kolega-ai-dev wants to merge 1 commit intobentoml:mainfrom
Open
security: validate alias paths in list_bento to prevent path traversal#1219kolega-ai-dev wants to merge 1 commit intobentoml:mainfrom
kolega-ai-dev wants to merge 1 commit intobentoml:mainfrom
Conversation
Alias files read by list_bento were used directly in path construction without validation. A malicious repo could use traversal sequences like '../' in bento.yaml version fields to escape the repo directory. Added checks to reject alias names containing '..' and verify the resolved path stays within the repo directory.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Vulnerability identified and fix provided by Kolega.dev
Path Traversal via Symlink Following in Alias Resolution
Location
src/openllm/model.py:159-163Description
The
list_bentofunction reads alias files without validating their contents. If an alias file contains a path like../../../etc/passwd, the resultingorigin_pathwill resolve outside the intended repo directory. No validation ensuresorigin_pathstays withinrepo.path.Analysis Notes
Confirmed path traversal vulnerability. The
list_bentofunction reads alias files at lines 159-163 and uses the content directly in path construction viaorigin_path = path.parent / origin_name. Alias files are created by_complete_alias()inrepo.pywhich writesbento.versionfrom the repository'sbento.yaml. A malicious repository can setversion: '../../../sensitive/path'inbento.yaml, causing the alias file to contain path traversal sequences. When the alias is read, no validation ensuresorigin_pathremains withinrepo.path. The resultingBentoInfoobject with a traversed path could be used to access files outside the intended repository directory. Exploitation requires adding a malicious repository viaopenllm repo add, which is a supported user operation.Fix Applied
Added two validation checks before constructing a
BentoInfofrom alias file contents: (1) reject alias values containing..path components usingpathlib.PurePosixPath.parts, and (2) verify the resolvedorigin_pathfalls within the repo directory usingresolve()andrelative_to(). Both checks silently skip invalid aliases viacontinue, matching the existing pattern of skipping entries that don't match expected structures.Tests/Linters Ran
ruff check src/openllm/model.py— passed (0 errors)ruff format --check src/openllm/model.py— passed (already formatted)mypy --strict --ignore-missing-imports src/openllm/model.py— no new errors (pre-existing stub-only warnings in dependencies)pytest --collect-only— no tests exist in the repository to runContribution Notes
DEVELOPMENT.mdpathlibusage consistent withrepo.pyandcommon.py