Skip to content

Commit e81151a

Browse files
committed
gh-139633: Run netrc file permission check only once per parse
Change the `.netrc` security check to be run once per parse of the default file rather than once per line inside the file.
1 parent 3195da0 commit e81151a

File tree

2 files changed

+48
-17
lines changed

2 files changed

+48
-17
lines changed

Lib/netrc.py

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -152,23 +152,28 @@ def _parse(self, file, fp, default_netrc):
152152
else:
153153
raise NetrcParseError("bad follower token %r" % tt,
154154
file, lexer.lineno)
155-
self._security_check(fp, default_netrc, self.hosts[entryname][0])
156-
157-
def _security_check(self, fp, default_netrc, login):
158-
if _can_security_check() and default_netrc and login != "anonymous":
159-
prop = os.fstat(fp.fileno())
160-
current_user_id = os.getuid()
161-
if prop.st_uid != current_user_id:
162-
fowner = _getpwuid(prop.st_uid)
163-
user = _getpwuid(current_user_id)
164-
raise NetrcParseError(
165-
f"~/.netrc file owner ({fowner}) does not match"
166-
f" current user ({user})")
167-
if (prop.st_mode & (stat.S_IRWXG | stat.S_IRWXO)):
168-
raise NetrcParseError(
169-
"~/.netrc access too permissive: access"
170-
" permissions must restrict access to only"
171-
" the owner")
155+
156+
if _can_security_check() and default_netrc:
157+
for entry in self.hosts.values():
158+
if entry[0] != "anonymous":
159+
# Raises on security issue
160+
self._security_check(fp)
161+
break
162+
163+
def _security_check(self, fp):
164+
prop = os.fstat(fp.fileno())
165+
current_user_id = os.getuid()
166+
if prop.st_uid != current_user_id:
167+
fowner = _getpwuid(prop.st_uid)
168+
user = _getpwuid(current_user_id)
169+
raise NetrcParseError(
170+
f"~/.netrc file owner ({fowner}) does not match"
171+
f" current user ({user})")
172+
if (prop.st_mode & (stat.S_IRWXG | stat.S_IRWXO)):
173+
raise NetrcParseError(
174+
"~/.netrc access too permissive: access"
175+
" permissions must restrict access to only"
176+
" the owner")
172177

173178
def authenticators(self, host):
174179
"""Return a (user, account, password) tuple for given host."""

Lib/test/test_netrc.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import netrc, os, unittest, sys, textwrap
2+
from pathlib import Path
23
from test import support
34
from test.support import os_helper
5+
from unittest.mock import Mock
6+
47

58
temp_filename = os_helper.TESTFN
69

@@ -309,6 +312,29 @@ def test_security(self):
309312
self.assertEqual(nrc.hosts['foo.domain.com'],
310313
('anonymous', '', 'pass'))
311314

315+
@unittest.skipUnless(os.name == 'posix', 'POSIX only test')
316+
@unittest.skipUnless(hasattr(os, 'getuid'), "os.getuid is required")
317+
@os_helper.skip_unless_working_chmod
318+
def test_security_only_once(self):
319+
# Make sure security check is only run once per parse when multiple
320+
# entries are found.
321+
check_called = netrc.netrc._security_check = Mock(return_value=True)
322+
323+
# Parse a default netrc with more than one password line.
324+
with os_helper.temp_dir() as tmp_dir:
325+
netrc_path = Path(tmp_dir) / '.netrc'
326+
netrc_path.write_text("""\
327+
machine foo.domain.com login bar password pass
328+
machine bar.domain.com login foo password pass
329+
""")
330+
netrc_path.chmod(0o600)
331+
with os_helper.EnvironmentVarGuard() as environ:
332+
environ.set('HOME', tmp_dir)
333+
netrc.netrc()
334+
335+
check_called.assert_called_once()
336+
del check_called
337+
312338

313339
if __name__ == "__main__":
314340
unittest.main()

0 commit comments

Comments
 (0)