Skip to content

Resolve "Path directory prefix bypass in ubifs/output.py" - #140

Open
qkaiser wants to merge 3 commits into
mainfrom
139-fix-path-traversal
Open

Resolve "Path directory prefix bypass in ubifs/output.py"#140
qkaiser wants to merge 3 commits into
mainfrom
139-fix-path-traversal

Conversation

@qkaiser

@qkaiser qkaiser commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

using path.startswith(basedir) without a trailing directory separator allows directory traversal if a target directory shares a common prefix.

For example, if basedir is /tmp/target, and path is ../target_hack, os.path.realpath resolves it to /tmp/target_hack, which passes the startswith check and allows writing outside the base directory.

Fixed by using the standardized implementation we use in jefferson. Implemented the test suite we use in unblob, and made sure we run the tests in CI.

Fixes #139

using path.startswith(basedir) without a trailing directory separator
allows directory traversal if a target directory shares a common prefix.

For example, if basedir is /tmp/target, and path is ../target_hack,
os.path.realpath resolves it to /tmp/target_hack, which passes the
startswith check and allows writing outside the base directory.

Fixed by using the standardized implementation we use in jefferson.
@qkaiser qkaiser self-assigned this Sep 4, 2026
@qkaiser qkaiser added bug python Pull requests that update python code labels Sep 4, 2026

@elektrischermoench elektrischermoench left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just worth mentioning test is passing on old code also.

("/lib/out", "some/dir/../../../", False),
("/lib/out", "some/dir/../../..", False),
("/lib/out", "../file", False),
("/lib/out", "/lib/out/../file", False),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is correct, but the test suite doesn't cover all effects of the bug. The old code accepted out_hack just because the name starts with out:

.venv/bin/python -m pytest tests/test_ubifs_output.py -v --no-cov
================================== test session starts ===================================
platform linux -- Python 3.14.7, pytest-9.1.1, pluggy-1.6.0 -- /tmp/claude-1000/-home-kai-development/281967f1-1e69-4fdb-beee-4285ab125719/scratchpad/ubi_reader/.venv/bin/python
cachedir: .pytest_cache
rootdir: /tmp/claude-1000/-home-kai-development/281967f1-1e69-4fdb-beee-4285ab125719/scratchpad/ubi_reader
configfile: pyproject.toml
plugins: cov-7.1.0
collected 11 items

tests/test_ubifs_output.py::test_is_safe_path[/lib/out-/lib/out/file-True] PASSED  [  9%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-file-True] PASSED           [ 18%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-dir/file-True] PASSED       [ 27%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-some/dir/file-True] PASSED  [ 36%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-some/dir/../file-True] PASSED [ 45%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-some/dir/../../file-True] PASSED [ 54%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-some/dir/../../../file-False] PASSED [ 63%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-some/dir/../../../-False] PASSED [ 72%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-some/dir/../../..-False] PASSED [ 81%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-../file-False] PASSED       [ 90%]
tests/test_ubifs_output.py::test_is_safe_path[/lib/out-/lib/out/../file-False] PASSED [100%]

=================================== 11 passed in 0.09s ===================================

Maybe add tests like this:

        ("/lib/out", "../out_hack", False),
        ("/lib/out", "../outx", False),
        ("/lib/out", "../out.bak/file", False),
        ("/lib/out", "/lib/out_hack", False),
        ("/lib/out", "/lib/outfile", False),
        ("/lib/out", "some/dir/../../../out_hack/file", False),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug python Pull requests that update python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Path directory prefix bypass in ubifs/output.py

2 participants