[issue6873] posix_lchown: possible overflow of uid, gid
Gregory P. Smith g...@krypto.org added the comment: a test would still be a good thing but this should be fixed regardless. -- resolution: - accepted status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Gregory P. Smith g...@krypto.org added the comment: I applied the same fix that was applied to chown in trunk r77007 for lchown and fchown. Could you test it on a platform where it previously failed? The existing code might still have issues if there are platforms where uid_t and gid_t are unsigned but not the same size as a long as at the moment it merely casts and does not test to see that the values are the same as the patch you have supplied here. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Boya Sun boya@case.edu added the comment: Gregory, I discovered this bug by static analysis, so I do not have a system that this bug is actually triggered. But I am happy to see the fix applied since this makes code safer. It would be great if anyone could write a test case that cause uid and gid to overflow, then use the test case as a regression test on the fix. Boya -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Changes by Gregory P. Smith g...@krypto.org: -- assignee: - gregory.p.smith nosy: +gregory.p.smith ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Changes by Gregory P. Smith g...@krypto.org: -- priority: - normal ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Boya Sun boya@case.edu added the comment: Martin, Corrected the patch accordingly. Can you verify whether the fix is correct or not now? Boya -- Added file: http://bugs.python.org/file14900/patch_6873.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Martin v. Löwis mar...@v.loewis.de added the comment: Yes, it looks correct now. I still wish it could be tested on a system where the problem actually occurs. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Martin v. Löwis mar...@v.loewis.de added the comment: You are saying that if I pass a long to the i argument parser it will cause a problem. But if I passed a int, it will be same as before and overflow will not be detected at all. Correct. So you should use the l argument parser. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Martin v. Löwis mar...@v.loewis.de added the comment: I think the new patch is still incorrect. You now pass long variables into the i argument parser. Also, I would expect that compilers prefer to see an explicit cast from long to uid_t, in case it's a truncating cast. Can you try your patch on a system where all this is an actual problem? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
STINNER Victor victor.stin...@haypocalc.com added the comment: @loewis: I don't think that the explicit cast is required. posix_setuid() has no explicit cast. But I would also prefer an explicit cast (just for the readability). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Boya Sun boya@case.edu added the comment: Martin, I am sorry that I do not have a system where this code actually triggered a problem, since this bug is discovered by a *static* analysis tool that is recently developed by our research group, which finds code segments that are similar to a previously fixed bugs as potential bugs. You are saying that if I pass a long to the i argument parser it will cause a problem. But if I passed a int, it will be same as before and overflow will not be detected at all. --- Victor, Do you also agree that it will cause a problem if I pass a long to the i argument parser? If so, I think maybe the overflow problem cannot be solved by the patch I submitted. Boya -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
STINNER Victor victor.stin...@haypocalc.com added the comment: posix modules contains a lot of function parsing uid_t / gid_t types. I would be nice to factorize the code: create a function to get an uid_t, and another to get a gid_t. I don't know the name of such callback, but it's used with: PyArg_ParseTuple(args, ...O..., ..., uid, get_uid, ...)). Such callbacks will be useful for: posix_chown(), posix_fchown(), posix_lchown(), posix_setuid(), posix_seteuid(), posix_setreuid(), posix_setegid(), posix_setregid(), posix_setgid(). And maybe also in: posix_setgroups(). In Python trunk, posix_set*id() function do check for uid_t/gid_d overflow, but not the posix_*chown() functions. The patch only fixes posix_lchown(). -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Martin v. Löwis mar...@v.loewis.de added the comment: The patch is incorrect. Why do you think there is an overflow? There is none in the call to ParseTuple: the i argument parser expects a signed int*; passing a long* will break on systems where sizeof(int)!=sizeof(long) (such as typical 64-bit Unix). In addition, the *actual* overflow in the current code (casting to uid_t) is not handled in the patch. -- nosy: +loewis ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Boya Sun boya@case.edu added the comment: Martin, The reason why I think there is a possible overflow is that according to issue 5705, uid/gid overflows are fixed in the following functions: posix_setegid, posix_setreuid(), posix_setregid(), posix_setgid(). So I think a similar fix should also be applied to the function posix_lchown. Or did I misunderstand anything? And you're right. The previous patch is incorrect. I now submitted another patch that deals with the *actual* overflow of gid and uid. --- Victor, I agree that all posix_*chown() functions should also be fixed for the same overflow problem, and it's a good idea to create callback functions as you described. But if nobody does that, I can at least created more patches to fix other posix_*chown() functions. -- Added file: http://bugs.python.org/file14873/patch_6873.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
Changes by Boya Sun boya@case.edu: Removed file: http://bugs.python.org/file14869/patch.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6873 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6873] posix_lchown: possible overflow of uid, gid
New submission from Boya Sun boya@case.edu:
posix_lchown(PyObject *self, PyObject *args)
{
...
int uid, gid;
...
if (!PyArg_ParseTuple(args, etii:lchown,
Py_FileSystemDefaultEncoding, path,
uid, gid))
...
}
uid and gid could cause over flow. A similar bug is issue 5705.
Patch attached. Any comment is appreciated!
Boya
--
files: patch.diff
keywords: patch
messages: 92465
nosy: boya
severity: normal
status: open
title: posix_lchown: possible overflow of uid, gid
Added file: http://bugs.python.org/file14869/patch.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6873
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
