Skip to content

Commit c97754d

Browse files
author
Samuel FORESTIER
committed
Manually resolves links relatively to root_dir, and prevent escape
This patch is a followup of #311. 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 own materials.
1 parent 65eda6f commit c97754d

File tree

11 files changed

+106
-7
lines changed

11 files changed

+106
-7
lines changed

src/distro/distro.py

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
"""
3030

3131
import argparse
32+
import errno
3233
import json
3334
import logging
3435
import os
@@ -744,6 +745,9 @@ def __init__(
744745
* :py:exc:`ValueError`: Initialization parameters combination is not
745746
supported.
746747
748+
* :py:exc:`PermissionError`: Execution has been halted as a path is
749+
leading to outside ``root_dir``, if specified.
750+
747751
* :py:exc:`OSError`: Some I/O issue with an os-release file or distro
748752
release file.
749753
@@ -766,8 +770,10 @@ def __init__(
766770

767771
# NOTE: The idea is to respect order **and** have it set
768772
# at all times for API backwards compatibility.
769-
if os.path.isfile(etc_dir_os_release_file) or not os.path.isfile(
770-
usr_lib_os_release_file
773+
if (
774+
self.root_dir is not None
775+
or os.path.isfile(etc_dir_os_release_file)
776+
or not os.path.isfile(usr_lib_os_release_file)
771777
):
772778
self.os_release_file = etc_dir_os_release_file
773779
else:
@@ -1078,6 +1084,53 @@ def uname_attr(self, attribute: str) -> str:
10781084
"""
10791085
return self._uname_info.get(attribute, "")
10801086

1087+
def __resolve_chroot_symlink(self, link_location: str) -> str:
1088+
# only resolve symlink if root_dir is set and link is under it
1089+
if (
1090+
self.root_dir is None
1091+
or os.path.commonprefix([self.root_dir, link_location]) != self.root_dir
1092+
):
1093+
return link_location
1094+
1095+
seen_paths = []
1096+
while True:
1097+
try:
1098+
resolved = os.readlink(link_location)
1099+
except OSError: # includes case "not a symlink"
1100+
if (
1101+
os.path.commonprefix(
1102+
[
1103+
os.path.realpath(self.root_dir),
1104+
os.path.realpath(link_location),
1105+
]
1106+
)
1107+
!= os.path.realpath(self.root_dir)
1108+
):
1109+
raise PermissionError(
1110+
f"{link_location} resolves outside of {self.root_dir}."
1111+
) from None
1112+
1113+
return link_location
1114+
1115+
if os.path.isabs(resolved):
1116+
# i.e. absolute path (regarding to the chroot), that we need to
1117+
# "move" back inside the chroot.
1118+
# But first we have to strip literals making this path absolute.
1119+
resolved = os.path.splitdrive(resolved)[1].lstrip(os.sep)
1120+
if os.altsep is not None:
1121+
resolved.lstrip(os.altsep)
1122+
resolved = os.path.join(self.root_dir, resolved)
1123+
else:
1124+
# i.e. relative path that we make absolute
1125+
resolved = os.path.join(os.path.dirname(link_location), resolved)
1126+
1127+
# prevent symlinks infinite loop
1128+
if resolved in seen_paths:
1129+
raise OSError(errno.ELOOP, os.strerror(errno.ELOOP), resolved)
1130+
1131+
seen_paths.append(link_location)
1132+
link_location = resolved
1133+
10811134
@cached_property
10821135
def _os_release_info(self) -> Dict[str, str]:
10831136
"""
@@ -1086,10 +1139,13 @@ def _os_release_info(self) -> Dict[str, str]:
10861139
Returns:
10871140
A dictionary containing all information items.
10881141
"""
1089-
if os.path.isfile(self.os_release_file):
1090-
with open(self.os_release_file, encoding="utf-8") as release_file:
1142+
try:
1143+
with open(
1144+
self.__resolve_chroot_symlink(self.os_release_file), encoding="utf-8"
1145+
) as release_file:
10911146
return self._parse_os_release_content(release_file)
1092-
return {}
1147+
except FileNotFoundError:
1148+
return {}
10931149

10941150
@staticmethod
10951151
def _parse_os_release_content(lines: TextIO) -> Dict[str, str]:
@@ -1254,7 +1310,11 @@ def _distro_release_info(self) -> Dict[str, str]:
12541310
basename
12551311
for basename in os.listdir(self.etc_dir)
12561312
if basename not in _DISTRO_RELEASE_IGNORE_BASENAMES
1257-
and os.path.isfile(os.path.join(self.etc_dir, basename))
1313+
and os.path.isfile(
1314+
self.__resolve_chroot_symlink(
1315+
os.path.join(self.etc_dir, basename)
1316+
)
1317+
)
12581318
]
12591319
# We sort for repeatability in cases where there are multiple
12601320
# distro specific files; e.g. CentOS, Oracle, Enterprise all
@@ -1301,7 +1361,7 @@ def _parse_distro_release_file(self, filepath: str) -> Dict[str, str]:
13011361
A dictionary containing all information items.
13021362
"""
13031363
try:
1304-
with open(filepath, encoding="utf-8") as fp:
1364+
with open(self.__resolve_chroot_symlink(filepath), encoding="utf-8") as fp:
13051365
# Only parse the first line. For instance, on SLES there
13061366
# are multiple lines. We don't want them...
13071367
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=absolutesymlinks
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+
/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
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ID=rootdirnonescape
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
os-release-symlink
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
os-release

0 commit comments

Comments
 (0)