Skip to content

Commit 7bcab19

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 8f5702c commit 7bcab19

12 files changed

Lines changed: 113 additions & 11 deletions

File tree

src/distro/distro.py

Lines changed: 76 additions & 11 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
@@ -768,8 +769,10 @@ def __init__(
768769

769770
# NOTE: The idea is to respect order **and** have it set
770771
# at all times for API backwards compatibility.
771-
if os.path.isfile(etc_dir_os_release_file) or not os.path.isfile(
772-
usr_lib_os_release_file
772+
if (
773+
self.root_dir is not None
774+
or os.path.isfile(etc_dir_os_release_file)
775+
or not os.path.isfile(usr_lib_os_release_file)
773776
):
774777
self.os_release_file = etc_dir_os_release_file
775778
else:
@@ -1083,6 +1086,59 @@ def uname_attr(self, attribute: str) -> str:
10831086
"""
10841087
return self._uname_info.get(attribute, "")
10851088

1089+
@staticmethod
1090+
def __abs_path_join(root_path: str, abs_path: str) -> str:
1091+
rel_path = os.path.splitdrive(abs_path)[1].lstrip(os.sep)
1092+
if os.altsep is not None:
1093+
rel_path = rel_path.lstrip(os.altsep)
1094+
1095+
return os.path.join(root_path, rel_path)
1096+
1097+
def __resolve_chroot_symlink_as_needed(self, link_location: str) -> str:
1098+
"""
1099+
Resolves a potential symlink in ``link_location``
1100+
against ``self.root_dir`` if inside the chroot,
1101+
else just return the original path.
1102+
We're doing this check at a central place, to making
1103+
the calling code more readable and to de-duplicate.
1104+
"""
1105+
if self.root_dir is None:
1106+
return link_location
1107+
1108+
if os.path.commonprefix([self.root_dir, link_location]) != self.root_dir:
1109+
raise FileNotFoundError
1110+
1111+
seen_paths = []
1112+
while True:
1113+
try:
1114+
resolved = os.readlink(link_location)
1115+
except OSError: # includes case "not a symlink"
1116+
if os.path.commonprefix(
1117+
[
1118+
os.path.realpath(self.root_dir),
1119+
os.path.realpath(link_location),
1120+
]
1121+
) != os.path.realpath(self.root_dir):
1122+
# `link_location` resolves outside of `self.root_dir`.
1123+
raise FileNotFoundError from None
1124+
1125+
return link_location
1126+
1127+
if os.path.isabs(resolved):
1128+
# i.e. absolute path (regarding to the chroot), that we need to
1129+
# "move" back inside the chroot.
1130+
resolved = self.__abs_path_join(self.root_dir, resolved)
1131+
else:
1132+
# i.e. relative path that we make absolute
1133+
resolved = os.path.join(os.path.dirname(link_location), resolved)
1134+
1135+
# prevent symlinks infinite loop
1136+
if resolved in seen_paths:
1137+
raise OSError(errno.ELOOP, os.strerror(errno.ELOOP), resolved)
1138+
1139+
seen_paths.append(link_location)
1140+
link_location = resolved
1141+
10861142
@cached_property
10871143
def _os_release_info(self) -> Dict[str, str]:
10881144
"""
@@ -1091,10 +1147,14 @@ def _os_release_info(self) -> Dict[str, str]:
10911147
Returns:
10921148
A dictionary containing all information items.
10931149
"""
1094-
if os.path.isfile(self.os_release_file):
1095-
with open(self.os_release_file, encoding="utf-8") as release_file:
1150+
try:
1151+
with open(
1152+
self.__resolve_chroot_symlink_as_needed(self.os_release_file),
1153+
encoding="utf-8",
1154+
) as release_file:
10961155
return self._parse_os_release_content(release_file)
1097-
return {}
1156+
except OSError:
1157+
return {}
10981158

10991159
@staticmethod
11001160
def _parse_os_release_content(lines: TextIO) -> Dict[str, str]:
@@ -1215,7 +1275,10 @@ def _oslevel_info(self) -> str:
12151275
def _debian_version(self) -> str:
12161276
try:
12171277
with open(
1218-
os.path.join(self.etc_dir, "debian_version"), encoding="ascii"
1278+
self.__resolve_chroot_symlink_as_needed(
1279+
os.path.join(self.etc_dir, "debian_version")
1280+
),
1281+
encoding="ascii",
12191282
) as fp:
12201283
return fp.readline().rstrip()
12211284
except FileNotFoundError:
@@ -1269,7 +1332,6 @@ def _distro_release_info(self) -> Dict[str, str]:
12691332
basename
12701333
for basename in os.listdir(self.etc_dir)
12711334
if basename not in _DISTRO_RELEASE_IGNORE_BASENAMES
1272-
and os.path.isfile(os.path.join(self.etc_dir, basename))
12731335
]
12741336
# We sort for repeatability in cases where there are multiple
12751337
# distro specific files; e.g. CentOS, Oracle, Enterprise all
@@ -1285,12 +1347,13 @@ def _distro_release_info(self) -> Dict[str, str]:
12851347
match = _DISTRO_RELEASE_BASENAME_PATTERN.match(basename)
12861348
if match is None:
12871349
continue
1288-
filepath = os.path.join(self.etc_dir, basename)
1289-
distro_info = self._parse_distro_release_file(filepath)
1350+
# NOTE: _parse_distro_release_file below will be resolving for us
1351+
unresolved_filepath = os.path.join(self.etc_dir, basename)
1352+
distro_info = self._parse_distro_release_file(unresolved_filepath)
12901353
# The name is always present if the pattern matches.
12911354
if "name" not in distro_info:
12921355
continue
1293-
self.distro_release_file = filepath
1356+
self.distro_release_file = unresolved_filepath
12941357
break
12951358
else: # the loop didn't "break": no candidate.
12961359
return {}
@@ -1316,7 +1379,9 @@ def _parse_distro_release_file(self, filepath: str) -> Dict[str, str]:
13161379
A dictionary containing all information items.
13171380
"""
13181381
try:
1319-
with open(filepath, encoding="utf-8") as fp:
1382+
with open(
1383+
self.__resolve_chroot_symlink_as_needed(filepath), encoding="utf-8"
1384+
) as fp:
13201385
# Only parse the first line. For instance, on SLES there
13211386
# are multiple lines. We don't want them...
13221387
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

tests/resources/testdistros/distro/rootdirnonescape/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=rootdirnonescape
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
os-release-symlink

0 commit comments

Comments
 (0)