|
| 1 | +From 62ad60773f29ed3bfc0f36bb6aa04ca4c51a0566 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Sindhu Karri <lakarri@microsoft.com> |
| 3 | +Date: Tue, 23 Jul 2024 08:54:49 +0000 |
| 4 | +Subject: [PATCH] Fix CVE-2024-6345 in python3-setuptools |
| 5 | + |
| 6 | +--- |
| 7 | + newsfragments/4332.feature.rst | 1 + |
| 8 | + setup.cfg | 1 + |
| 9 | + setuptools/package_index.py | 146 ++++++++++++++------------ |
| 10 | + setuptools/tests/test_packageindex.py | 56 +++++----- |
| 11 | + 4 files changed, 108 insertions(+), 96 deletions(-) |
| 12 | + create mode 100644 newsfragments/4332.feature.rst |
| 13 | + |
| 14 | +diff --git a/newsfragments/4332.feature.rst b/newsfragments/4332.feature.rst |
| 15 | +new file mode 100644 |
| 16 | +index 0000000..9f46298 |
| 17 | +--- /dev/null |
| 18 | ++++ b/newsfragments/4332.feature.rst |
| 19 | +@@ -0,0 +1 @@ |
| 20 | ++Modernized and refactored VCS handling in package_index. |
| 21 | +\ No newline at end of file |
| 22 | +diff --git a/setup.cfg b/setup.cfg |
| 23 | +index 4e12e70..7797a9f 100644 |
| 24 | +--- a/setup.cfg |
| 25 | ++++ b/setup.cfg |
| 26 | +@@ -67,6 +67,7 @@ testing = |
| 27 | + pytest-perf; \ |
| 28 | + sys_platform != "cygwin" |
| 29 | + jaraco.develop >= 7.21; python_version >= "3.9" and sys_platform != "cygwin" |
| 30 | ++ pytest-subprocess |
| 31 | + testing-integration = |
| 32 | + pytest |
| 33 | + pytest-xdist |
| 34 | +diff --git a/setuptools/package_index.py b/setuptools/package_index.py |
| 35 | +index 3cedd51..cf25f83 100644 |
| 36 | +--- a/setuptools/package_index.py |
| 37 | ++++ b/setuptools/package_index.py |
| 38 | +@@ -1,6 +1,7 @@ |
| 39 | + """PyPI and direct package downloading.""" |
| 40 | + |
| 41 | + import sys |
| 42 | ++import subprocess |
| 43 | + import os |
| 44 | + import re |
| 45 | + import io |
| 46 | +@@ -586,7 +587,7 @@ class PackageIndex(Environment): |
| 47 | + scheme = URL_SCHEME(spec) |
| 48 | + if scheme: |
| 49 | + # It's a url, download it to tmpdir |
| 50 | +- found = self._download_url(scheme.group(1), spec, tmpdir) |
| 51 | ++ found = self._download_url(spec, tmpdir) |
| 52 | + base, fragment = egg_info_for_url(spec) |
| 53 | + if base.endswith('.py'): |
| 54 | + found = self.gen_setup(found, fragment, tmpdir) |
| 55 | +@@ -812,7 +813,7 @@ class PackageIndex(Environment): |
| 56 | + else: |
| 57 | + raise DistutilsError("Download error for %s: %s" % (url, v)) from v |
| 58 | + |
| 59 | +- def _download_url(self, scheme, url, tmpdir): |
| 60 | ++ def _download_url(self, url, tmpdir): |
| 61 | + # Determine download filename |
| 62 | + # |
| 63 | + name, fragment = egg_info_for_url(url) |
| 64 | +@@ -827,19 +828,59 @@ class PackageIndex(Environment): |
| 65 | + |
| 66 | + filename = os.path.join(tmpdir, name) |
| 67 | + |
| 68 | +- # Download the file |
| 69 | +- # |
| 70 | +- if scheme == 'svn' or scheme.startswith('svn+'): |
| 71 | +- return self._download_svn(url, filename) |
| 72 | +- elif scheme == 'git' or scheme.startswith('git+'): |
| 73 | +- return self._download_git(url, filename) |
| 74 | +- elif scheme.startswith('hg+'): |
| 75 | +- return self._download_hg(url, filename) |
| 76 | +- elif scheme == 'file': |
| 77 | +- return urllib.request.url2pathname(urllib.parse.urlparse(url)[2]) |
| 78 | +- else: |
| 79 | +- self.url_ok(url, True) # raises error if not allowed |
| 80 | +- return self._attempt_download(url, filename) |
| 81 | ++ return self._download_vcs(url, filename) or self._download_other(url, filename) |
| 82 | ++ |
| 83 | ++ @staticmethod |
| 84 | ++ def _resolve_vcs(url): |
| 85 | ++ """ |
| 86 | ++ >>> rvcs = PackageIndex._resolve_vcs |
| 87 | ++ >>> rvcs('git+http://foo/bar') |
| 88 | ++ 'git' |
| 89 | ++ >>> rvcs('hg+https://foo/bar') |
| 90 | ++ 'hg' |
| 91 | ++ >>> rvcs('git:myhost') |
| 92 | ++ 'git' |
| 93 | ++ >>> rvcs('hg:myhost') |
| 94 | ++ >>> rvcs('http://foo/bar') |
| 95 | ++ """ |
| 96 | ++ scheme = urllib.parse.urlsplit(url).scheme |
| 97 | ++ pre, sep, post = scheme.partition('+') |
| 98 | ++ # svn and git have their own protocol; hg does not |
| 99 | ++ allowed = set(['svn', 'git'] + ['hg'] * bool(sep)) |
| 100 | ++ return next(iter({pre} & allowed), None) |
| 101 | ++ |
| 102 | ++ def _download_vcs(self, url, spec_filename): |
| 103 | ++ vcs = self._resolve_vcs(url) |
| 104 | ++ if not vcs: |
| 105 | ++ return |
| 106 | ++ if vcs == 'svn': |
| 107 | ++ raise DistutilsError( |
| 108 | ++ f"Invalid config, SVN download is not supported: {url}" |
| 109 | ++ ) |
| 110 | ++ |
| 111 | ++ filename, _, _ = spec_filename.partition('#') |
| 112 | ++ url, rev = self._vcs_split_rev_from_url(url) |
| 113 | ++ |
| 114 | ++ self.info(f"Doing {vcs} clone from {url} to {filename}") |
| 115 | ++ subprocess.check_call([vcs, 'clone', '--quiet', url, filename]) |
| 116 | ++ |
| 117 | ++ co_commands = dict( |
| 118 | ++ git=[vcs, '-C', filename, 'checkout', '--quiet', rev], |
| 119 | ++ hg=[vcs, '--cwd', filename, 'up', '-C', '-r', rev, '-q'], |
| 120 | ++ ) |
| 121 | ++ if rev is not None: |
| 122 | ++ self.info(f"Checking out {rev}") |
| 123 | ++ subprocess.check_call(co_commands[vcs]) |
| 124 | ++ |
| 125 | ++ return filename |
| 126 | ++ |
| 127 | ++ def _download_other(self, url, filename): |
| 128 | ++ scheme = urllib.parse.urlsplit(url).scheme |
| 129 | ++ if scheme == 'file': # pragma: no cover |
| 130 | ++ return urllib.request.url2pathname(urllib.parse.urlparse(url).path) |
| 131 | ++ # raise error if not allowed |
| 132 | ++ self.url_ok(url, True) |
| 133 | ++ return self._attempt_download(url, filename) |
| 134 | + |
| 135 | + def scan_url(self, url): |
| 136 | + self.process_url(url, True) |
| 137 | +@@ -855,64 +896,37 @@ class PackageIndex(Environment): |
| 138 | + os.unlink(filename) |
| 139 | + raise DistutilsError(f"Unexpected HTML page found at {url}") |
| 140 | + |
| 141 | +- def _download_svn(self, url, _filename): |
| 142 | +- raise DistutilsError(f"Invalid config, SVN download is not supported: {url}") |
| 143 | +- |
| 144 | + @staticmethod |
| 145 | +- def _vcs_split_rev_from_url(url, pop_prefix=False): |
| 146 | +- scheme, netloc, path, query, frag = urllib.parse.urlsplit(url) |
| 147 | ++ def _vcs_split_rev_from_url(url): |
| 148 | ++ """ |
| 149 | ++ Given a possible VCS URL, return a clean URL and resolved revision if any. |
| 150 | ++ |
| 151 | ++ >>> vsrfu = PackageIndex._vcs_split_rev_from_url |
| 152 | ++ >>> vsrfu('git+https://github.com/pypa/setuptools@v69.0.0#egg-info=setuptools') |
| 153 | ++ ('https://github.com/pypa/setuptools', 'v69.0.0') |
| 154 | ++ >>> vsrfu('git+https://github.com/pypa/setuptools#egg-info=setuptools') |
| 155 | ++ ('https://github.com/pypa/setuptools', None) |
| 156 | ++ >>> vsrfu('http://foo/bar') |
| 157 | ++ ('http://foo/bar', None) |
| 158 | ++ """ |
| 159 | ++ parts = urllib.parse.urlsplit(url) |
| 160 | + |
| 161 | +- scheme = scheme.split('+', 1)[-1] |
| 162 | ++ clean_scheme = parts.scheme.split('+', 1)[-1] |
| 163 | + |
| 164 | + # Some fragment identification fails |
| 165 | +- path = path.split('#', 1)[0] |
| 166 | +- |
| 167 | +- rev = None |
| 168 | +- if '@' in path: |
| 169 | +- path, rev = path.rsplit('@', 1) |
| 170 | +- |
| 171 | +- # Also, discard fragment |
| 172 | +- url = urllib.parse.urlunsplit((scheme, netloc, path, query, '')) |
| 173 | +- |
| 174 | +- return url, rev |
| 175 | +- |
| 176 | +- def _download_git(self, url, filename): |
| 177 | +- filename = filename.split('#', 1)[0] |
| 178 | +- url, rev = self._vcs_split_rev_from_url(url, pop_prefix=True) |
| 179 | +- |
| 180 | +- self.info("Doing git clone from %s to %s", url, filename) |
| 181 | +- os.system("git clone --quiet %s %s" % (url, filename)) |
| 182 | +- |
| 183 | +- if rev is not None: |
| 184 | +- self.info("Checking out %s", rev) |
| 185 | +- os.system( |
| 186 | +- "git -C %s checkout --quiet %s" |
| 187 | +- % ( |
| 188 | +- filename, |
| 189 | +- rev, |
| 190 | +- ) |
| 191 | +- ) |
| 192 | ++ no_fragment_path, _, _ = parts.path.partition('#') |
| 193 | + |
| 194 | +- return filename |
| 195 | ++ pre, sep, post = no_fragment_path.rpartition('@') |
| 196 | ++ clean_path, rev = (pre, post) if sep else (post, None) |
| 197 | + |
| 198 | +- def _download_hg(self, url, filename): |
| 199 | +- filename = filename.split('#', 1)[0] |
| 200 | +- url, rev = self._vcs_split_rev_from_url(url, pop_prefix=True) |
| 201 | ++ resolved = parts._replace( |
| 202 | ++ scheme=clean_scheme, |
| 203 | ++ path=clean_path, |
| 204 | ++ # discard the fragment |
| 205 | ++ fragment='', |
| 206 | ++ ).geturl() |
| 207 | + |
| 208 | +- self.info("Doing hg clone from %s to %s", url, filename) |
| 209 | +- os.system("hg clone --quiet %s %s" % (url, filename)) |
| 210 | +- |
| 211 | +- if rev is not None: |
| 212 | +- self.info("Updating to %s", rev) |
| 213 | +- os.system( |
| 214 | +- "hg --cwd %s up -C -r %s -q" |
| 215 | +- % ( |
| 216 | +- filename, |
| 217 | +- rev, |
| 218 | +- ) |
| 219 | +- ) |
| 220 | +- |
| 221 | +- return filename |
| 222 | ++ return resolved, rev |
| 223 | + |
| 224 | + def debug(self, msg, *args): |
| 225 | + log.debug(msg, *args) |
| 226 | +diff --git a/setuptools/tests/test_packageindex.py b/setuptools/tests/test_packageindex.py |
| 227 | +index 0287063..82f4382 100644 |
| 228 | +--- a/setuptools/tests/test_packageindex.py |
| 229 | ++++ b/setuptools/tests/test_packageindex.py |
| 230 | +@@ -5,7 +5,6 @@ import platform |
| 231 | + import urllib.request |
| 232 | + import urllib.error |
| 233 | + import http.client |
| 234 | +-from unittest import mock |
| 235 | + |
| 236 | + import pytest |
| 237 | + |
| 238 | +@@ -186,49 +185,46 @@ class TestPackageIndex: |
| 239 | + assert dists[0].version == '' |
| 240 | + assert dists[1].version == vc |
| 241 | + |
| 242 | +- def test_download_git_with_rev(self, tmpdir): |
| 243 | ++ def test_download_git_with_rev(self, tmp_path, fp): |
| 244 | + url = 'git+https://github.example/group/project@master#egg=foo' |
| 245 | + index = setuptools.package_index.PackageIndex() |
| 246 | + |
| 247 | +- with mock.patch("os.system") as os_system_mock: |
| 248 | +- result = index.download(url, str(tmpdir)) |
| 249 | ++ expected_dir = tmp_path / 'project@master' |
| 250 | ++ fp.register([ |
| 251 | ++ 'git', |
| 252 | ++ 'clone', |
| 253 | ++ '--quiet', |
| 254 | ++ 'https://github.example/group/project', |
| 255 | ++ expected_dir, |
| 256 | ++ ]) |
| 257 | ++ fp.register(['git', '-C', expected_dir, 'checkout', '--quiet', 'master']) |
| 258 | + |
| 259 | +- os_system_mock.assert_called() |
| 260 | ++ result = index.download(url, tmp_path) |
| 261 | + |
| 262 | +- expected_dir = str(tmpdir / 'project@master') |
| 263 | +- expected = ( |
| 264 | +- 'git clone --quiet ' 'https://github.example/group/project {expected_dir}' |
| 265 | +- ).format(**locals()) |
| 266 | +- first_call_args = os_system_mock.call_args_list[0][0] |
| 267 | +- assert first_call_args == (expected,) |
| 268 | ++ assert result == str(expected_dir) |
| 269 | ++ assert len(fp.calls) == 2 |
| 270 | + |
| 271 | +- tmpl = 'git -C {expected_dir} checkout --quiet master' |
| 272 | +- expected = tmpl.format(**locals()) |
| 273 | +- assert os_system_mock.call_args_list[1][0] == (expected,) |
| 274 | +- assert result == expected_dir |
| 275 | +- |
| 276 | +- def test_download_git_no_rev(self, tmpdir): |
| 277 | ++ def test_download_git_no_rev(self, tmp_path, fp): |
| 278 | + url = 'git+https://github.example/group/project#egg=foo' |
| 279 | + index = setuptools.package_index.PackageIndex() |
| 280 | + |
| 281 | +- with mock.patch("os.system") as os_system_mock: |
| 282 | +- result = index.download(url, str(tmpdir)) |
| 283 | +- |
| 284 | +- os_system_mock.assert_called() |
| 285 | +- |
| 286 | +- expected_dir = str(tmpdir / 'project') |
| 287 | +- expected = ( |
| 288 | +- 'git clone --quiet ' 'https://github.example/group/project {expected_dir}' |
| 289 | +- ).format(**locals()) |
| 290 | +- os_system_mock.assert_called_once_with(expected) |
| 291 | +- |
| 292 | +- def test_download_svn(self, tmpdir): |
| 293 | ++ expected_dir = tmp_path / 'project' |
| 294 | ++ fp.register([ |
| 295 | ++ 'git', |
| 296 | ++ 'clone', |
| 297 | ++ '--quiet', |
| 298 | ++ 'https://github.example/group/project', |
| 299 | ++ expected_dir, |
| 300 | ++ ]) |
| 301 | ++ index.download(url, tmp_path) |
| 302 | ++ |
| 303 | ++ def test_download_svn(self, tmp_path): |
| 304 | + url = 'svn+https://svn.example/project#egg=foo' |
| 305 | + index = setuptools.package_index.PackageIndex() |
| 306 | + |
| 307 | + msg = r".*SVN download is not supported.*" |
| 308 | + with pytest.raises(distutils.errors.DistutilsError, match=msg): |
| 309 | +- index.download(url, str(tmpdir)) |
| 310 | ++ index.download(url, tmp_path) |
| 311 | + |
| 312 | + |
| 313 | + class TestContentCheckers: |
| 314 | +-- |
| 315 | +2.33.8 |
| 316 | + |
0 commit comments