Issue 7365: grp and pwd should treat uid and gid as unsigned (original) (raw)

Both Linux and Solaris define uid_t and gid_t as unsigned integers. The pwd and grp modules cast these to signed long values that are then converted with PyInt_FromLong. For large values, greater than 2 ** 32 - 1, the result is correct when Python is compiled as a 64-bit executable, but is incorrect when compiled as a 32-bit executable.

Similar bugs have been noted in the posix module as reported in #4591.

For example, on OpenSolaris build 127, the 32-bit version of Python returns a negative uid: pw_uid=-2147483647:

$ file /usr/bin/python2.6 /usr/bin/python2.6: ELF 32-bit LSB executable 80386 Version 1 [FPU], dynamically linked, not stripped, no debugging information available

$ /usr/bin/python2.6 Python 2.6.2 (r262, Oct 26 2009, 01:06:14) [C] on sunos5 Type "help", "copyright", "credits" or "license" for more information.

import pwd pwd.getpwnam('test@foo.com') pwd.struct_passwd(pw_name='test@foo.com', pw_passwd='x', pw_uid=-2147483647, pw_gid=10000, pw_gecos='Test User', pw_dir='', pw_shell='')

$ file /usr/bin/amd64/python2.6 /usr/bin/amd64/python2.6: ELF 64-bit LSB executable AMD64 Version 1 [SSE FXSR FPU], dynamically linked, not stripped, no debugging information available

$ /usr/bin/amd64/python2.6 Python 2.6.2 (r262, Oct 26 2009, 01:09:04) [C] on sunos5 Type "help", "copyright", "credits" or "license" for more information.

import pwd pwd.getpwnam('test@foo.com') pwd.struct_passwd(pw_name='test@foo.com', pw_passwd='x', pw_uid=2147483649, pw_gid=10000, pw_gecos='Test User', pw_dir='', pw_shell='')

The attached patch against 2.6.4 changes PyInt_FromLong to PyLong_FromUnsignedLong and changes casts to unsigned long.

I took the freedom to refresh the patch against default (I don't know yet the policies for what to backport and what not), built a debug version of python with the patch applied and run the test suite with no regression.

A test would be nice to be added, but I'm not sure how to do that (is test_grp.py reading local /etc/groups or so?).

I'd be happy another pair of eyes to look at the patch, but it seems worth to be committing it (modulo the backport thing: what lower version should be targetting?).