Skip to content

Commit 50f1498

Browse files
committed
Manually resolve paths relatively to root_dir to prevent escape
This patch is a followup of #311 (2a89f76). It appeared that we were not resolving paths when reading from files. This means that symbolic links present under `root_dir` could be blindly followed, eventually leading _outside_ of `root_dir` (i.e host own files). Note : this patch **only** changes LinuxDistribution `root_dir` parameter behavior.
1 parent 211dc76 commit 50f1498

File tree

14 files changed

+192
-12
lines changed

14 files changed

+192
-12
lines changed

src/distro/distro.py

Lines changed: 129 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import subprocess
3838
import sys
3939
import warnings
40+
from pathlib import Path
4041
from typing import (
4142
Any,
4243
Callable,
@@ -795,9 +796,15 @@ def __init__(
795796
self.usr_lib_dir, _OS_RELEASE_BASENAME
796797
)
797798

799+
def __isfile(path: str) -> bool:
800+
try:
801+
return os.path.isfile(self._resolve_path_relatively_to_chroot(path))
802+
except FileNotFoundError:
803+
return False
804+
798805
# NOTE: The idea is to respect order **and** have it set
799806
# at all times for API backwards compatibility.
800-
if os.path.isfile(etc_dir_os_release_file) or not os.path.isfile(
807+
if __isfile(etc_dir_os_release_file) or not __isfile(
801808
usr_lib_os_release_file
802809
):
803810
self.os_release_file = etc_dir_os_release_file
@@ -1152,6 +1159,101 @@ def _uname_attr(self, attribute: str) -> str:
11521159
"""
11531160
return self._uname_info.get(attribute, "")
11541161

1162+
@staticmethod
1163+
def __abs_path_join(root_path: Path, abs_path: Path) -> Path:
1164+
rel_path = os.path.splitdrive(abs_path)[1].lstrip(os.sep)
1165+
if os.altsep is not None:
1166+
rel_path = rel_path.lstrip(os.altsep)
1167+
1168+
return root_path / Path(rel_path)
1169+
1170+
def _resolve_path_relatively_to_chroot(self, path: str) -> Path:
1171+
"""
1172+
Resolves any encountered symbolic links in ``path`` relatively to
1173+
``self.root_dir``, if defined. Otherwise it would simply return
1174+
original ``path``.
1175+
This function could be considered as a "soft-chroot" implementation.
1176+
We're doing this check at a central place, to make calling code more readable
1177+
and to de-duplicate.
1178+
1179+
Raises:
1180+
1181+
* :py:exc:`FileNotFoundError`: ``path`` doesn't resolve in chroot, or resolving
1182+
it lead to symbolic links loop
1183+
1184+
Examples :
1185+
1186+
* if root_dir="/path/to/chroot" and path="folder/../../../../etc/os-release"
1187+
with "etc" resolving to "/mnt/disk/etc" and "os-release" to
1188+
"../../usr/lib/os-release", this function returns
1189+
"/path/to/chroot/mnt/usr/lib/os-release"
1190+
1191+
* if root_dir=None and path="/path/to/os-release", this function returns
1192+
"/path/to/os-release"
1193+
"""
1194+
path_to_resolve = Path(path)
1195+
1196+
if self.root_dir is None:
1197+
return path_to_resolve
1198+
1199+
# resolve `self.root_dir` once and for all
1200+
chroot_path = Path(self.root_dir).resolve()
1201+
1202+
# consider non-absolute `path_to_resolve` relative to chroot
1203+
if not path_to_resolve.is_absolute():
1204+
path_to_resolve = chroot_path / path_to_resolve
1205+
1206+
seen_paths = set()
1207+
while True:
1208+
# although `path_to_resolve` _should_ be relative to chroot (either
1209+
# passed from trusted code or already resolved by previous loop
1210+
# iteration), we enforce this check as some inputs are available through API
1211+
try:
1212+
relative_parts = path_to_resolve.relative_to(chroot_path).parts
1213+
except ValueError:
1214+
raise FileNotFoundError
1215+
1216+
# iterate over (relative) path segments and try to resolve each one of them
1217+
for i, part in enumerate(relative_parts, start=1):
1218+
if part == os.pardir:
1219+
# normalize path parts up to this segment (relatively to chroot)
1220+
path_to_resolve = self.__abs_path_join(
1221+
chroot_path,
1222+
Path(os.path.normpath("/" / Path(*relative_parts[:i]))),
1223+
) / Path(*relative_parts[i:])
1224+
break # restart path resolution as path has just been normalized
1225+
1226+
# attempt symbolic link resolution on current path segment
1227+
symlink_candidate = chroot_path / Path(*relative_parts[:i])
1228+
try:
1229+
symlink_resolved = Path(os.readlink(symlink_candidate))
1230+
except (
1231+
AttributeError, # `readlink` isn't supported by system
1232+
OSError, # not a symlink, go to next path segment
1233+
):
1234+
continue
1235+
1236+
# "bend" **absolute** resolved path inside the chroot
1237+
# consider **non-absolute** resolved path relatively to chroot
1238+
if symlink_resolved.is_absolute():
1239+
path_to_resolve = self.__abs_path_join(
1240+
chroot_path, symlink_resolved
1241+
)
1242+
else:
1243+
path_to_resolve = symlink_candidate.parent / symlink_resolved
1244+
1245+
# append remaining path segments to resolved path
1246+
path_to_resolve /= Path(*relative_parts[i:])
1247+
break # restart path resolution as a symlink has just been resolved
1248+
else:
1249+
# `path_to_resolve` can be considered resolved, return it
1250+
return path_to_resolve
1251+
1252+
# prevent symlinks infinite loop by tracking successive resolutions
1253+
if path_to_resolve in seen_paths:
1254+
raise FileNotFoundError
1255+
seen_paths.add(path_to_resolve)
1256+
11551257
@cached_property
11561258
def _os_release_info(self) -> Dict[str, str]:
11571259
"""
@@ -1160,10 +1262,14 @@ def _os_release_info(self) -> Dict[str, str]:
11601262
Returns:
11611263
A dictionary containing all information items.
11621264
"""
1163-
if os.path.isfile(self.os_release_file):
1164-
with open(self.os_release_file, encoding="utf-8") as release_file:
1265+
try:
1266+
with open(
1267+
self._resolve_path_relatively_to_chroot(self.os_release_file),
1268+
encoding="utf-8",
1269+
) as release_file:
11651270
return self._parse_os_release_content(release_file)
1166-
return {}
1271+
except FileNotFoundError:
1272+
return {}
11671273

11681274
@staticmethod
11691275
def _parse_os_release_content(lines: TextIO) -> Dict[str, str]:
@@ -1286,7 +1392,10 @@ def _oslevel_info(self) -> str:
12861392
def _debian_version(self) -> str:
12871393
try:
12881394
with open(
1289-
os.path.join(self.etc_dir, "debian_version"), encoding="ascii"
1395+
self._resolve_path_relatively_to_chroot(
1396+
os.path.join(self.etc_dir, "debian_version")
1397+
),
1398+
encoding="ascii",
12901399
) as fp:
12911400
return fp.readline().rstrip()
12921401
except FileNotFoundError:
@@ -1296,7 +1405,10 @@ def _debian_version(self) -> str:
12961405
def _armbian_version(self) -> str:
12971406
try:
12981407
with open(
1299-
os.path.join(self.etc_dir, "armbian-release"), encoding="ascii"
1408+
self._resolve_path_relatively_to_chroot(
1409+
os.path.join(self.etc_dir, "armbian-release")
1410+
),
1411+
encoding="ascii",
13001412
) as fp:
13011413
return self._parse_os_release_content(fp).get("version", "")
13021414
except FileNotFoundError:
@@ -1348,9 +1460,10 @@ def _distro_release_info(self) -> Dict[str, str]:
13481460
try:
13491461
basenames = [
13501462
basename
1351-
for basename in os.listdir(self.etc_dir)
1463+
for basename in os.listdir(
1464+
self._resolve_path_relatively_to_chroot(self.etc_dir)
1465+
)
13521466
if basename not in _DISTRO_RELEASE_IGNORE_BASENAMES
1353-
and os.path.isfile(os.path.join(self.etc_dir, basename))
13541467
]
13551468
# We sort for repeatability in cases where there are multiple
13561469
# distro specific files; e.g. CentOS, Oracle, Enterprise all
@@ -1366,12 +1479,13 @@ def _distro_release_info(self) -> Dict[str, str]:
13661479
match = _DISTRO_RELEASE_BASENAME_PATTERN.match(basename)
13671480
if match is None:
13681481
continue
1369-
filepath = os.path.join(self.etc_dir, basename)
1370-
distro_info = self._parse_distro_release_file(filepath)
1482+
# NOTE: _parse_distro_release_file below will be resolving for us
1483+
unresolved_filepath = os.path.join(self.etc_dir, basename)
1484+
distro_info = self._parse_distro_release_file(unresolved_filepath)
13711485
# The name is always present if the pattern matches.
13721486
if "name" not in distro_info:
13731487
continue
1374-
self.distro_release_file = filepath
1488+
self.distro_release_file = unresolved_filepath
13751489
break
13761490
else: # the loop didn't "break": no candidate.
13771491
return {}
@@ -1405,7 +1519,10 @@ def _parse_distro_release_file(self, filepath: str) -> Dict[str, str]:
14051519
A dictionary containing all information items.
14061520
"""
14071521
try:
1408-
with open(filepath, encoding="utf-8") as fp:
1522+
with open(
1523+
self._resolve_path_relatively_to_chroot(filepath),
1524+
encoding="utf-8",
1525+
) as fp:
14091526
# Only parse the first line. For instance, on SLES there
14101527
# are multiple lines. We don't want them...
14111528
return self._parse_distro_release_content(fp.readline())
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/tmp/another-link
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/usr/lib/os-release
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ID=absolute_symlinks
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../../../distros/debian8/etc/os-release
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../usr/lib/os-release
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/tmp/another-link/../../../../../../usr/lib/os-release
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
nested/nested

tests/resources/testdistros/distro/root_dir_non_escape/tmp/nested/nested/.gitkeep

Whitespace-only changes.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ID=root_dir_non_escape

0 commit comments

Comments
 (0)