From 9f12d468f4f886d70831b34875e95d5efe9c6323 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Wed, 23 Dec 2009 09:31:11 +0000 Subject: Fix possible integer overflow in lchown and fchown functions. For issue1747858. --- Lib/test/test_posix.py | 73 +++++++++++++++++++++++++++++++++----------------- Modules/posixmodule.c | 9 ++++--- 2 files changed, 53 insertions(+), 29 deletions(-) diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index c6da157..9d631db 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -207,32 +207,55 @@ class PosixTester(unittest.TestCase): if hasattr(posix, 'stat'): self.assertTrue(posix.stat(test_support.TESTFN)) - if hasattr(posix, 'chown'): - def test_chown(self): - # raise an OSError if the file does not exist - os.unlink(test_support.TESTFN) - self.assertRaises(OSError, posix.chown, test_support.TESTFN, -1, -1) - - # re-create the file - open(test_support.TESTFN, 'w').close() - if os.getuid() == 0: - try: - # Many linux distros have a nfsnobody user as MAX_UID-2 - # that makes a good test case for signedness issues. - # http://bugs.python.org/issue1747858 - # This part of the test only runs when run as root. - # Only scary people run their tests as root. - ent = pwd.getpwnam('nfsnobody') - posix.chown(test_support.TESTFN, ent.pw_uid, ent.pw_gid) - except KeyError: - pass - else: - # non-root cannot chown to root, raises OSError - self.assertRaises(OSError, posix.chown, - test_support.TESTFN, 0, 0) + def _test_all_chown_common(self, chown_func, first_param): + """Common code for chown, fchown and lchown tests.""" + if os.getuid() == 0: + try: + # Many linux distros have a nfsnobody user as MAX_UID-2 + # that makes a good test case for signedness issues. + # http://bugs.python.org/issue1747858 + # This part of the test only runs when run as root. + # Only scary people run their tests as root. + ent = pwd.getpwnam('nfsnobody') + chown_func(first_param, ent.pw_uid, ent.pw_gid) + except KeyError: + pass + else: + # non-root cannot chown to root, raises OSError + self.assertRaises(OSError, chown_func, + first_param, 0, 0) + + # test a successful chown call + chown_func(first_param, os.getuid(), os.getgid()) + + @unittest.skipUnless(hasattr(posix, 'chown'), "test needs os.chown()") + def test_chown(self): + # raise an OSError if the file does not exist + os.unlink(test_support.TESTFN) + self.assertRaises(OSError, posix.chown, test_support.TESTFN, -1, -1) + + # re-create the file + open(test_support.TESTFN, 'w').close() + self._test_all_chown_common(posix.chown, test_support.TESTFN) - # test a successful chown call - posix.chown(test_support.TESTFN, os.getuid(), os.getgid()) + @unittest.skipUnless(hasattr(posix, 'fchown'), "test needs os.fchown()") + def test_fchown(self): + os.unlink(test_support.TESTFN) + + # re-create the file + test_file = open(test_support.TESTFN, 'w') + try: + fd = test_file.fileno() + self._test_all_chown_common(posix.fchown, fd) + finally: + test_file.close() + + @unittest.skipUnless(hasattr(posix, 'lchown'), "test needs os.lchown()") + def test_lchown(self): + os.unlink(test_support.TESTFN) + # create a symlink + os.symlink('/tmp/dummy-symlink-target', test_support.TESTFN) + self._test_all_chown_common(posix.lchown, test_support.TESTFN) def test_chdir(self): if hasattr(posix, 'chdir'): diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 8956e93..76609ee 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -1910,9 +1910,10 @@ fd to the numeric uid and gid."); static PyObject * posix_fchown(PyObject *self, PyObject *args) { - int fd, uid, gid; + int fd; + long uid, gid; int res; - if (!PyArg_ParseTuple(args, "iii:chown", &fd, &uid, &gid)) + if (!PyArg_ParseTuple(args, "ill:chown", &fd, &uid, &gid)) return NULL; Py_BEGIN_ALLOW_THREADS res = fchown(fd, (uid_t) uid, (gid_t) gid); @@ -1933,9 +1934,9 @@ static PyObject * posix_lchown(PyObject *self, PyObject *args) { char *path = NULL; - int uid, gid; + long uid, gid; int res; - if (!PyArg_ParseTuple(args, "etii:lchown", + if (!PyArg_ParseTuple(args, "etll:lchown", Py_FileSystemDefaultEncoding, &path, &uid, &gid)) return NULL; -- cgit v0.12