Skip to content

Commit dc19990

Browse files
HorlogeSkynetHorlogeSkynet
committed
Manually resolves links relatively to root_dir, and 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 a symbolic link present under `root_dir` could be blindly followed _outside_ of `root_dir`, possibly leading to host files. Note : this patch **only** changes `root_dir` behavior.
1 parent 791fc0d commit dc19990

File tree

14 files changed

+158
-11
lines changed

14 files changed

+158
-11
lines changed

‎src/distro/distro.py‎

Lines changed: 95 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,76 @@ def uname_attr(self, attribute: str) -> str:
10911091
"""
10921092
return self._uname_info.get(attribute, "")
10931093

1094+
@staticmethod
1095+
def __abs_path_join(root_path: str, abs_path: str) -> str:
1096+
rel_path = os.path.splitdrive(abs_path)[1].lstrip(os.sep)
1097+
if os.altsep is not None:
1098+
rel_path = rel_path.lstrip(os.altsep)
1099+
1100+
return os.path.join(root_path, rel_path)
1101+
1102+
def __resolve_chroot_symlink_as_needed(self, link_location: str) -> str:
1103+
"""
1104+
Resolves a potential symlink in ``link_location`` against
1105+
``self.root_dir`` if inside the chroot, else just return the original
1106+
path.
1107+
We're doing this check at a central place, to making the calling code
1108+
more readable and to de-duplicate.
1109+
"""
1110+
if self.root_dir is None:
1111+
return link_location
1112+
1113+
# resolve `self.root_dir`, once and for all.
1114+
root_dir = os.path.realpath(self.root_dir)
1115+
1116+
# consider non-absolute `link_location` relative to `root_dir` (as
1117+
# `os.path.commonpath` does not support mixing absolute and relative
1118+
# paths).
1119+
if not os.path.isabs(link_location):
1120+
link_location = self.__abs_path_join(root_dir, link_location)
1121+
1122+
seen_paths = set()
1123+
while True:
1124+
# while `link_location` _should_ be relative to chroot (either
1125+
# passed from trusted code or already resolved by previous loop
1126+
# iteration), we enforce this check as `self.os_release_file` and
1127+
# `self.distro_release_file` may be user-supplied.
1128+
if os.path.commonpath([root_dir, link_location]) != root_dir:
1129+
raise FileNotFoundError
1130+
1131+
if not os.path.islink(link_location):
1132+
# assert _final_ path is actually inside chroot (this is
1133+
# required to address `..` usages, potentially leading to
1134+
# outside, after subsequent link resolutions).
1135+
if (
1136+
os.path.commonpath([root_dir, os.path.realpath(link_location)])
1137+
!= root_dir
1138+
):
1139+
raise FileNotFoundError
1140+
1141+
return link_location
1142+
1143+
resolved = os.readlink(link_location)
1144+
if not os.path.isabs(resolved):
1145+
# compute resolved path relatively to previous `link_location`
1146+
# and accordingly to chroot. We also canonize "top" `..`
1147+
# components (relatively to `root_dir`), as they would
1148+
# legitimately resolve to chroot itself).
1149+
resolved = os.path.relpath(
1150+
os.path.join(os.path.dirname(link_location), resolved),
1151+
start=root_dir,
1152+
).lstrip(os.pardir + os.pathsep)
1153+
1154+
# "move" back (absolute) path inside the chroot
1155+
resolved = self.__abs_path_join(root_dir, resolved)
1156+
1157+
# prevent symlinks infinite loop
1158+
if resolved in seen_paths:
1159+
raise FileNotFoundError
1160+
1161+
seen_paths.add(link_location)
1162+
link_location = resolved
1163+
10941164
@cached_property
10951165
def _os_release_info(self) -> Dict[str, str]:
10961166
"""
@@ -1099,10 +1169,14 @@ def _os_release_info(self) -> Dict[str, str]:
10991169
Returns:
11001170
A dictionary containing all information items.
11011171
"""
1102-
if os.path.isfile(self.os_release_file):
1103-
with open(self.os_release_file, encoding="utf-8") as release_file:
1172+
try:
1173+
with open(
1174+
self.__resolve_chroot_symlink_as_needed(self.os_release_file),
1175+
encoding="utf-8",
1176+
) as release_file:
11041177
return self._parse_os_release_content(release_file)
1105-
return {}
1178+
except FileNotFoundError:
1179+
return {}
11061180

11071181
@staticmethod
11081182
def _parse_os_release_content(lines: TextIO) -> Dict[str, str]:
@@ -1223,7 +1297,10 @@ def _oslevel_info(self) -> str:
12231297
def _debian_version(self) -> str:
12241298
try:
12251299
with open(
1226-
os.path.join(self.etc_dir, "debian_version"), encoding="ascii"
1300+
self.__resolve_chroot_symlink_as_needed(
1301+
os.path.join(self.etc_dir, "debian_version")
1302+
),
1303+
encoding="ascii",
12271304
) as fp:
12281305
return fp.readline().rstrip()
12291306
except FileNotFoundError:
@@ -1233,7 +1310,10 @@ def _debian_version(self) -> str:
12331310
def _armbian_version(self) -> str:
12341311
try:
12351312
with open(
1236-
os.path.join(self.etc_dir, "armbian-release"), encoding="ascii"
1313+
self.__resolve_chroot_symlink_as_needed(
1314+
os.path.join(self.etc_dir, "armbian-release")
1315+
),
1316+
encoding="ascii",
12371317
) as fp:
12381318
return self._parse_os_release_content(fp).get("version", "")
12391319
except FileNotFoundError:
@@ -1285,9 +1365,10 @@ def _distro_release_info(self) -> Dict[str, str]:
12851365
try:
12861366
basenames = [
12871367
basename
1288-
for basename in os.listdir(self.etc_dir)
1368+
for basename in os.listdir(
1369+
self.__resolve_chroot_symlink_as_needed(self.etc_dir)
1370+
)
12891371
if basename not in _DISTRO_RELEASE_IGNORE_BASENAMES
1290-
and os.path.isfile(os.path.join(self.etc_dir, basename))
12911372
]
12921373
# We sort for repeatability in cases where there are multiple
12931374
# distro specific files; e.g. CentOS, Oracle, Enterprise all
@@ -1303,12 +1384,13 @@ def _distro_release_info(self) -> Dict[str, str]:
13031384
match = _DISTRO_RELEASE_BASENAME_PATTERN.match(basename)
13041385
if match is None:
13051386
continue
1306-
filepath = os.path.join(self.etc_dir, basename)
1307-
distro_info = self._parse_distro_release_file(filepath)
1387+
# NOTE: _parse_distro_release_file below will be resolving for us
1388+
unresolved_filepath = os.path.join(self.etc_dir, basename)
1389+
distro_info = self._parse_distro_release_file(unresolved_filepath)
13081390
# The name is always present if the pattern matches.
13091391
if "name" not in distro_info:
13101392
continue
1311-
self.distro_release_file = filepath
1393+
self.distro_release_file = unresolved_filepath
13121394
break
13131395
else: # the loop didn't "break": no candidate.
13141396
return {}
@@ -1342,7 +1424,9 @@ def _parse_distro_release_file(self, filepath: str) -> Dict[str, str]:
13421424
A dictionary containing all information items.
13431425
"""
13441426
try:
1345-
with open(filepath, encoding="utf-8") as fp:
1427+
with open(
1428+
self.__resolve_chroot_symlink_as_needed(filepath), encoding="utf-8"
1429+
) as fp:
13461430
# Only parse the first line. For instance, on SLES there
13471431
# are multiple lines. We don't want them...
13481432
return self._parse_distro_release_content(fp.readline())

‎tests/resources/testdistros/distro/root_dir_absolute_symlinks/etc/os-release‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/tmp/another-link

‎tests/resources/testdistros/distro/root_dir_absolute_symlinks/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

‎tests/resources/testdistros/distro/root_dir_absolute_symlinks/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

‎tests/resources/testdistros/distro/root_dir_escape/etc/os-release‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../../../distros/debian8/etc/os-release

‎tests/resources/testdistros/distro/root_dir_escape_abs/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

‎tests/resources/testdistros/distro/root_dir_non_escape/etc/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

‎tests/resources/testdistros/distro/root_dir_non_escape/tmp/another-link‎

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.

‎tests/resources/testdistros/distro/root_dir_non_escape/usr/lib/os-release‎

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)