mirror of
https://github.com/langflow-ai/langflow.git
synced 2026-07-24 07:02:37 +08:00
fix(security): reject symlinks/hardlinks in BaseFileComponent TAR extraction (GHSA-ccv6-r384-xp75) (#12945)
`BaseFileComponent._unpack_bundle._safe_extract_tar` accepted any TAR member type and only checked that `output_dir / member.name` did not escape the extract dir. That check was performed before extraction, so a symlink whose *target* was an absolute path (or `../` escape) was extracted untouched. Once on disk the link was iterated by `temp_dir_path.iterdir()` and handed to `process_files()`, whose concrete implementations (FileComponent, DoclingInline/Remote, NvidiaIngest, VideoFile, Unstructured) call `path.read_bytes()` and follow the link to read arbitrary host files. The reporter's exploit chain leaks `~/.langflow/secret_key`, forges a JWT for an admin user, and then runs arbitrary code through the Python interpreter node, achieving RCE. Python's `tarfile` only defaults to the safe `data` filter on Python 3.14, which langflow's `requires-python = ">=3.10,<3.14"` excludes — so every supported interpreter was vulnerable. Fix: - `_safe_extract_tar` now rejects symbolic-link, hard-link, FIFO, and device-node members with a `ValueError` and only extracts regular files and directories. - `_unpack_and_collect_files` skips any `is_symlink()` entries from the extracted bundle directory and from recursive directory walks as defense-in-depth in case a future bundle format slips a link through. - New `tests/unit/base/data/test_base_file_unpack.py` covers symlink (abs + relative escape), hardlink, FIFO rejection, benign tar/zip extraction, the post-extraction symlink filter, and an end-to-end repro mirroring the advisory PoC (real filesystem symlink → tarfile.add). Refs: https://github.com/langflow-ai/langflow/security/advisories/GHSA-ccv6-r384-xp75
This commit is contained in:
@ -702,7 +702,10 @@ class BaseFileComponent(Component, ABC):
|
||||
data = file.data
|
||||
|
||||
if path.is_dir():
|
||||
# Recurse into directories
|
||||
# Recurse into directories. Skip symlinks defensively so that a
|
||||
# link planted in a previously-extracted bundle (or a directory
|
||||
# the user pointed at) cannot be dereferenced into an arbitrary
|
||||
# host file (GHSA-ccv6-r384-xp75).
|
||||
collected_files.extend(
|
||||
[
|
||||
BaseFileComponent.BaseFile(
|
||||
@ -711,7 +714,7 @@ class BaseFileComponent(Component, ABC):
|
||||
delete_after_processing=delete_after_processing,
|
||||
)
|
||||
for sub_path in path.rglob("*")
|
||||
if sub_path.is_file()
|
||||
if sub_path.is_file() and not sub_path.is_symlink()
|
||||
]
|
||||
)
|
||||
elif path.suffix[1:] in self.SUPPORTED_BUNDLE_EXTENSIONS:
|
||||
@ -720,7 +723,11 @@ class BaseFileComponent(Component, ABC):
|
||||
self._temp_dirs.append(temp_dir)
|
||||
temp_dir_path = Path(temp_dir.name)
|
||||
self._unpack_bundle(path, temp_dir_path)
|
||||
subpaths = list(temp_dir_path.iterdir())
|
||||
# Drop any symlink that may have slipped through extraction.
|
||||
# `_unpack_bundle` rejects link members for TAR archives, but
|
||||
# this guard keeps the contract in place for any future bundle
|
||||
# type added to SUPPORTED_BUNDLE_EXTENSIONS.
|
||||
subpaths = [p for p in temp_dir_path.iterdir() if not p.is_symlink()]
|
||||
self.log(f"Unpacked bundle {path.name} into {subpaths}")
|
||||
collected_files.extend(
|
||||
[
|
||||
@ -768,11 +775,24 @@ class BaseFileComponent(Component, ABC):
|
||||
bundle.extract(member, path=output_dir)
|
||||
|
||||
def _safe_extract_tar(bundle: tarfile.TarFile, output_dir: Path):
|
||||
"""Safely extract TAR files."""
|
||||
"""Safely extract TAR files.
|
||||
|
||||
Only regular files and directories are extracted. Symlinks, hardlinks,
|
||||
and device/FIFO members are rejected because they could be made to
|
||||
point at arbitrary locations on the host filesystem and lead to
|
||||
arbitrary file read once the extracted entries are subsequently
|
||||
ingested by `process_files()` (GHSA-ccv6-r384-xp75).
|
||||
"""
|
||||
for member in bundle.getmembers():
|
||||
# Filter out resource fork information for automatic production of mac
|
||||
if Path(member.name).name.startswith("._"):
|
||||
continue
|
||||
if member.issym() or member.islnk():
|
||||
msg = f"Refusing to extract link member from TAR File: {member.name!r} -> {member.linkname!r}"
|
||||
raise ValueError(msg)
|
||||
if not (member.isfile() or member.isdir()):
|
||||
msg = f"Refusing to extract non-regular TAR member: {member.name!r}"
|
||||
raise ValueError(msg)
|
||||
member_path = output_dir / member.name
|
||||
# Ensure no path traversal outside `output_dir`
|
||||
if not member_path.resolve().is_relative_to(output_dir.resolve()):
|
||||
|
||||
236
src/lfx/tests/unit/base/data/test_base_file_unpack.py
Normal file
236
src/lfx/tests/unit/base/data/test_base_file_unpack.py
Normal file
@ -0,0 +1,236 @@
|
||||
"""Security regression tests for BaseFileComponent bundle extraction.
|
||||
|
||||
Covers GHSA-ccv6-r384-xp75: a TAR member that is a symlink, hardlink, or device
|
||||
node could be made to point at an arbitrary host file. When the extracted entry
|
||||
is later read by `process_files()` the host file's contents would be ingested
|
||||
into the downstream sink. The fix in `_safe_extract_tar` rejects every member
|
||||
that is not a regular file or directory, and `_unpack_and_collect_files` skips
|
||||
any symlinks defensively before handing entries to `process_files()`.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
import tarfile
|
||||
import zipfile
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
from lfx.base.data.base_file import BaseFileComponent
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
class _StubFileComponent(BaseFileComponent):
|
||||
"""Minimal concrete subclass used to exercise the unpack helpers."""
|
||||
|
||||
VALID_EXTENSIONS = ["txt"]
|
||||
|
||||
def __init__(self, **data):
|
||||
super().__init__(**data)
|
||||
self.set_attributes(
|
||||
{
|
||||
"path": [],
|
||||
"file_path": None,
|
||||
"separator": "\n\n",
|
||||
"silent_errors": False,
|
||||
"delete_server_file_after_processing": True,
|
||||
"ignore_unsupported_extensions": True,
|
||||
"ignore_unspecified_files": False,
|
||||
}
|
||||
)
|
||||
|
||||
def process_files(self, file_list): # pragma: no cover - not exercised here
|
||||
return file_list
|
||||
|
||||
|
||||
def _add_file(tar: tarfile.TarFile, name: str, payload: bytes) -> None:
|
||||
info = tarfile.TarInfo(name=name)
|
||||
info.size = len(payload)
|
||||
info.type = tarfile.REGTYPE
|
||||
tar.addfile(info, io.BytesIO(payload))
|
||||
|
||||
|
||||
def _add_symlink(tar: tarfile.TarFile, name: str, target: str) -> None:
|
||||
info = tarfile.TarInfo(name=name)
|
||||
info.type = tarfile.SYMTYPE
|
||||
info.linkname = target
|
||||
tar.addfile(info)
|
||||
|
||||
|
||||
def _add_hardlink(tar: tarfile.TarFile, name: str, target: str) -> None:
|
||||
info = tarfile.TarInfo(name=name)
|
||||
info.type = tarfile.LNKTYPE
|
||||
info.linkname = target
|
||||
tar.addfile(info)
|
||||
|
||||
|
||||
def _add_fifo(tar: tarfile.TarFile, name: str) -> None:
|
||||
info = tarfile.TarInfo(name=name)
|
||||
info.type = tarfile.FIFOTYPE
|
||||
tar.addfile(info)
|
||||
|
||||
|
||||
def _build_tar(tmp_path: Path, name: str, populate) -> Path:
|
||||
bundle = tmp_path / name
|
||||
with tarfile.open(bundle, "w") as tar:
|
||||
populate(tar)
|
||||
return bundle
|
||||
|
||||
|
||||
def _build_zip(tmp_path: Path, name: str, files: dict[str, bytes]) -> Path:
|
||||
bundle = tmp_path / name
|
||||
with zipfile.ZipFile(bundle, "w") as zf:
|
||||
for member, payload in files.items():
|
||||
zf.writestr(member, payload)
|
||||
return bundle
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def component() -> _StubFileComponent:
|
||||
return _StubFileComponent()
|
||||
|
||||
|
||||
def test_tar_with_absolute_symlink_is_rejected(tmp_path, component):
|
||||
"""A symlink whose target is an absolute host path must be refused."""
|
||||
secret = tmp_path / "secret.txt"
|
||||
secret.write_bytes(b"jwt-signing-secret")
|
||||
|
||||
bundle = _build_tar(
|
||||
tmp_path,
|
||||
"evil.tar",
|
||||
lambda tar: _add_symlink(tar, "leak", str(secret)),
|
||||
)
|
||||
|
||||
extract_dir = tmp_path / "out_abs"
|
||||
extract_dir.mkdir()
|
||||
with pytest.raises(ValueError, match="Refusing to extract link member"):
|
||||
component._unpack_bundle(bundle, extract_dir)
|
||||
assert list(extract_dir.iterdir()) == []
|
||||
|
||||
|
||||
def test_tar_with_relative_escape_symlink_is_rejected(tmp_path, component):
|
||||
"""A symlink that uses ../ to escape the extract dir must be refused."""
|
||||
bundle = _build_tar(
|
||||
tmp_path,
|
||||
"escape.tar",
|
||||
lambda tar: _add_symlink(tar, "leak", "../../etc/passwd"),
|
||||
)
|
||||
|
||||
extract_dir = tmp_path / "out_rel"
|
||||
extract_dir.mkdir()
|
||||
with pytest.raises(ValueError, match="Refusing to extract link member"):
|
||||
component._unpack_bundle(bundle, extract_dir)
|
||||
assert list(extract_dir.iterdir()) == []
|
||||
|
||||
|
||||
def test_tar_with_hardlink_is_rejected(tmp_path, component):
|
||||
"""Hardlinks have the same arbitrary-target risk as symlinks."""
|
||||
|
||||
def populate(tar):
|
||||
_add_file(tar, "real.txt", b"ok")
|
||||
_add_hardlink(tar, "leak", "../etc/passwd")
|
||||
|
||||
bundle = _build_tar(tmp_path, "hardlink.tar", populate)
|
||||
|
||||
extract_dir = tmp_path / "out_hl"
|
||||
extract_dir.mkdir()
|
||||
with pytest.raises(ValueError, match="Refusing to extract link member"):
|
||||
component._unpack_bundle(bundle, extract_dir)
|
||||
|
||||
|
||||
def test_tar_with_fifo_member_is_rejected(tmp_path, component):
|
||||
"""Non-regular members (FIFO/device) must be refused."""
|
||||
bundle = _build_tar(tmp_path, "fifo.tar", lambda tar: _add_fifo(tar, "pipe"))
|
||||
|
||||
extract_dir = tmp_path / "out_fifo"
|
||||
extract_dir.mkdir()
|
||||
with pytest.raises(ValueError, match="Refusing to extract non-regular TAR member"):
|
||||
component._unpack_bundle(bundle, extract_dir)
|
||||
|
||||
|
||||
def test_tar_with_only_regular_files_extracts(tmp_path, component):
|
||||
"""The fix must not regress benign archives."""
|
||||
|
||||
def populate(tar):
|
||||
_add_file(tar, "a.txt", b"alpha")
|
||||
_add_file(tar, "nested/b.txt", b"beta")
|
||||
|
||||
bundle = _build_tar(tmp_path, "ok.tar", populate)
|
||||
|
||||
extract_dir = tmp_path / "out_ok"
|
||||
extract_dir.mkdir()
|
||||
component._unpack_bundle(bundle, extract_dir)
|
||||
|
||||
assert (extract_dir / "a.txt").read_bytes() == b"alpha"
|
||||
assert (extract_dir / "nested" / "b.txt").read_bytes() == b"beta"
|
||||
|
||||
|
||||
def test_zip_with_only_regular_files_extracts(tmp_path, component):
|
||||
"""ZIP path must remain working unchanged."""
|
||||
bundle = _build_zip(tmp_path, "ok.zip", {"a.txt": b"alpha", "nested/b.txt": b"beta"})
|
||||
|
||||
extract_dir = tmp_path / "out_zip"
|
||||
extract_dir.mkdir()
|
||||
component._unpack_bundle(bundle, extract_dir)
|
||||
|
||||
assert (extract_dir / "a.txt").read_bytes() == b"alpha"
|
||||
assert (extract_dir / "nested" / "b.txt").read_bytes() == b"beta"
|
||||
|
||||
|
||||
def test_collect_files_skips_symlinks_in_extracted_dir(tmp_path, component):
|
||||
"""Defense-in-depth check for the post-extraction iteration.
|
||||
|
||||
A symlink that somehow lands in an unpacked dir must not be passed to
|
||||
``process_files()``. Simulated by manually planting one, since
|
||||
``_safe_extract_tar`` would otherwise refuse it.
|
||||
"""
|
||||
extract_root = tmp_path / "extracted"
|
||||
extract_root.mkdir()
|
||||
real = extract_root / "doc.txt"
|
||||
real.write_bytes(b"hello")
|
||||
secret = tmp_path / "secret.txt"
|
||||
secret.write_bytes(b"top-secret")
|
||||
(extract_root / "leak").symlink_to(secret)
|
||||
|
||||
files = [BaseFileComponent.BaseFile(data=None, path=extract_root)]
|
||||
collected = component._unpack_and_collect_files(files)
|
||||
|
||||
paths = {bf.path.name for bf in collected}
|
||||
assert "doc.txt" in paths
|
||||
assert "leak" not in paths
|
||||
|
||||
|
||||
def test_unpack_bundle_rejects_unsupported_format(tmp_path, component):
|
||||
"""An input that is neither zip nor tar still raises clearly."""
|
||||
bogus = tmp_path / "not-a-bundle.bin"
|
||||
bogus.write_bytes(b"not an archive")
|
||||
|
||||
extract_dir = tmp_path / "out_bogus"
|
||||
extract_dir.mkdir()
|
||||
with pytest.raises(ValueError, match="Unsupported bundle format"):
|
||||
component._unpack_bundle(bogus, extract_dir)
|
||||
|
||||
|
||||
def test_real_filesystem_symlink_in_a_tar_via_tarfile_is_rejected(tmp_path, component):
|
||||
"""End-to-end repro of the advisory's PoC archive shape.
|
||||
|
||||
Builds the tar from a real filesystem symlink (the way the reporter's
|
||||
archive was produced) and confirms extraction is refused.
|
||||
"""
|
||||
target = tmp_path / "host_secret"
|
||||
target.write_bytes(b"x")
|
||||
workdir = tmp_path / "src"
|
||||
workdir.mkdir()
|
||||
(workdir / "leak").symlink_to(target)
|
||||
|
||||
bundle = tmp_path / "from_fs.tar"
|
||||
with tarfile.open(bundle, "w") as tar:
|
||||
tar.add(workdir / "leak", arcname="leak")
|
||||
|
||||
extract_dir = tmp_path / "out_fs"
|
||||
extract_dir.mkdir()
|
||||
with pytest.raises(ValueError, match="Refusing to extract link member"):
|
||||
component._unpack_bundle(bundle, extract_dir)
|
||||
assert list(extract_dir.iterdir()) == []
|
||||
Reference in New Issue
Block a user