Skip to content

Commit 1cdfc44

Browse files
Samuel FORESTIERHorlogeSkynet
authored andcommitted
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 files.
1 parent 7ce285c commit 1cdfc44

14 files changed

Lines changed: 150 additions & 14 deletions

File tree

src/distro/distro.py

Lines changed: 90 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,7 @@ def __init__(
757757
* :py:exc:`UnicodeError`: A data source has unexpected characters or
758758
uses an unexpected encoding.
759759
"""
760-
self.root_dir = root_dir
760+
self.root_dir = os.path.realpath(root_dir) if root_dir else None
761761
self.etc_dir = os.path.join(root_dir, "etc") if root_dir else _UNIXCONFDIR
762762
self.usr_lib_dir = (
763763
os.path.join(root_dir, "usr/lib") if root_dir else _UNIXUSRLIBDIR
@@ -773,8 +773,10 @@ def __init__(
773773

774774
# NOTE: The idea is to respect order **and** have it set
775775
# at all times for API backwards compatibility.
776-
if os.path.isfile(etc_dir_os_release_file) or not os.path.isfile(
777-
usr_lib_os_release_file
776+
if (
777+
self.root_dir is not None
778+
or os.path.isfile(etc_dir_os_release_file)
779+
or not os.path.isfile(usr_lib_os_release_file)
778780
):
779781
self.os_release_file = etc_dir_os_release_file
780782
else:
@@ -1091,6 +1093,66 @@ def uname_attr(self, attribute: str) -> str:
10911093
"""
10921094
return self._uname_info.get(attribute, "")
10931095

1096+
@staticmethod
1097+
def __abs_path_join(root_path: str, abs_path: str) -> str:
1098+
rel_path = os.path.splitdrive(abs_path)[1].lstrip(os.sep)
1099+
if os.altsep is not None:
1100+
rel_path = rel_path.lstrip(os.altsep)
1101+
1102+
return os.path.join(root_path, rel_path)
1103+
1104+
def __resolve_chroot_symlink_as_needed(self, link_location: str) -> str:
1105+
"""
1106+
Resolves a potential symlink in ``link_location`` against
1107+
``self.root_dir`` if inside the chroot, else just return the original
1108+
path.
1109+
We're doing this check at a central place, to making the calling code
1110+
more readable and to de-duplicate.
1111+
"""
1112+
if self.root_dir is None:
1113+
return link_location
1114+
1115+
# consider non-absolute `link_location` relative to `root_dir` (as
1116+
# `os.path.commonpath` does not support mixing absolute and relative
1117+
# paths).
1118+
if not os.path.isabs(link_location):
1119+
link_location = self.__abs_path_join(self.root_dir, link_location)
1120+
1121+
seen_paths = set()
1122+
while True:
1123+
# while `link_location` _should_ be relative to chroot (either
1124+
# passed from trusted code or already resolved by previous loop
1125+
# iteration), we enforce this check as `self.os_release_file` and
1126+
# `self.distro_release_file` may be user-supplied.
1127+
if os.path.commonpath(
1128+
[self.root_dir, link_location]
1129+
) != self.root_dir.rstrip(os.sep):
1130+
raise FileNotFoundError
1131+
1132+
if not os.path.islink(link_location):
1133+
return link_location
1134+
1135+
resolved = os.readlink(link_location)
1136+
if not os.path.isabs(resolved):
1137+
# compute resolved path relatively to previous `link_location`
1138+
# and accordingly to chroot. We also canonize "top" `..`
1139+
# components (relatively to `self.root_dir`), as they would
1140+
# legitimately resolve to chroot itself).
1141+
resolved = os.path.relpath(
1142+
os.path.join(os.path.dirname(link_location), resolved),
1143+
start=self.root_dir,
1144+
).lstrip(os.pardir + os.pathsep)
1145+
1146+
# "move" back absolute path inside the chroot
1147+
resolved = self.__abs_path_join(self.root_dir, resolved)
1148+
1149+
# prevent symlinks infinite loop
1150+
if resolved in seen_paths:
1151+
raise FileNotFoundError
1152+
1153+
seen_paths.add(link_location)
1154+
link_location = resolved
1155+
10941156
@cached_property
10951157
def _os_release_info(self) -> Dict[str, str]:
10961158
"""
@@ -1099,10 +1161,14 @@ def _os_release_info(self) -> Dict[str, str]:
10991161
Returns:
11001162
A dictionary containing all information items.
11011163
"""
1102-
if os.path.isfile(self.os_release_file):
1103-
with open(self.os_release_file, encoding="utf-8") as release_file:
1164+
try:
1165+
with open(
1166+
self.__resolve_chroot_symlink_as_needed(self.os_release_file),
1167+
encoding="utf-8",
1168+
) as release_file:
11041169
return self._parse_os_release_content(release_file)
1105-
return {}
1170+
except OSError:
1171+
return {}
11061172

11071173
@staticmethod
11081174
def _parse_os_release_content(lines: TextIO) -> Dict[str, str]:
@@ -1223,7 +1289,10 @@ def _oslevel_info(self) -> str:
12231289
def _debian_version(self) -> str:
12241290
try:
12251291
with open(
1226-
os.path.join(self.etc_dir, "debian_version"), encoding="ascii"
1292+
self.__resolve_chroot_symlink_as_needed(
1293+
os.path.join(self.etc_dir, "debian_version")
1294+
),
1295+
encoding="ascii",
12271296
) as fp:
12281297
return fp.readline().rstrip()
12291298
except FileNotFoundError:
@@ -1233,7 +1302,10 @@ def _debian_version(self) -> str:
12331302
def _armbian_version(self) -> str:
12341303
try:
12351304
with open(
1236-
os.path.join(self.etc_dir, "armbian-release"), encoding="ascii"
1305+
self.__resolve_chroot_symlink_as_needed(
1306+
os.path.join(self.etc_dir, "armbian-release")
1307+
),
1308+
encoding="ascii",
12371309
) as fp:
12381310
return self._parse_os_release_content(fp).get("version", "")
12391311
except FileNotFoundError:
@@ -1285,9 +1357,10 @@ def _distro_release_info(self) -> Dict[str, str]:
12851357
try:
12861358
basenames = [
12871359
basename
1288-
for basename in os.listdir(self.etc_dir)
1360+
for basename in os.listdir(
1361+
self.__resolve_chroot_symlink_as_needed(self.etc_dir)
1362+
)
12891363
if basename not in _DISTRO_RELEASE_IGNORE_BASENAMES
1290-
and os.path.isfile(os.path.join(self.etc_dir, basename))
12911364
]
12921365
# We sort for repeatability in cases where there are multiple
12931366
# distro specific files; e.g. CentOS, Oracle, Enterprise all
@@ -1303,12 +1376,13 @@ def _distro_release_info(self) -> Dict[str, str]:
13031376
match = _DISTRO_RELEASE_BASENAME_PATTERN.match(basename)
13041377
if match is None:
13051378
continue
1306-
filepath = os.path.join(self.etc_dir, basename)
1307-
distro_info = self._parse_distro_release_file(filepath)
1379+
# NOTE: _parse_distro_release_file below will be resolving for us
1380+
unresolved_filepath = os.path.join(self.etc_dir, basename)
1381+
distro_info = self._parse_distro_release_file(unresolved_filepath)
13081382
# The name is always present if the pattern matches.
13091383
if "name" not in distro_info:
13101384
continue
1311-
self.distro_release_file = filepath
1385+
self.distro_release_file = unresolved_filepath
13121386
break
13131387
else: # the loop didn't "break": no candidate.
13141388
return {}
@@ -1342,7 +1416,9 @@ def _parse_distro_release_file(self, filepath: str) -> Dict[str, str]:
13421416
A dictionary containing all information items.
13431417
"""
13441418
try:
1345-
with open(filepath, encoding="utf-8") as fp:
1419+
with open(
1420+
self.__resolve_chroot_symlink_as_needed(filepath), encoding="utf-8"
1421+
) as fp:
13461422
# Only parse the first line. For instance, on SLES there
13471423
# are multiple lines. We don't want them...
13481424
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+
/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
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ID=root_dir_os_release_file_abs

0 commit comments

Comments
 (0)