This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: sysmodule.c: realpath() is unsafe
Type: security Stage: needs patch
Components: Interpreter Core Versions: Python 3.2, Python 3.3, Python 3.4, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, ajaksu2, christian.heimes, matejcik, mibanescu, misa, pitrou, pjones, spektrum, vstinner
Priority: normal Keywords: patch

Created on 2005-09-22 14:54 by misa, last changed 2022-04-11 14:56 by admin.

Files
File name Uploaded Description Edit
python-canonicalize.patch misa, 2005-09-22 14:54 Patch to make Python use canonicalize_file_name if available
Messages (13)
msg48753 - (view) Author: Mihai Ibanescu (misa) Date: 2005-09-22 14:54
realpath() will dereference all symlinks and resolve
references to /./ and /../ (and so on). realpath
accepts a source buffer and a destination buffer to
copy the resolved path into. On certain systems
PATH_MAX can be of arbitrary size, while the buffer
passed in would be of a limiited size.

There is no way to specify how long your "resolved"
buffer is, therefore it is possible for one to overflow it.

According to the man page:

BUGS
        Never  use this function. It is broken by
design since it is impossible
        to determine a suitable size for the output
buffer.  According to POSIX
        a  buffer of size PATH_MAX suffices, but
PATH_MAX need not be a defined
        constant, and may have to be obtained  using 
pathconf().   And  asking
        pathconf() does not really help, since on the
one hand POSIX warns that
        the result of pathconf() may be huge and
unsuitable for mallocing  mem-
        ory.  And  on  the  other hand pathconf() may
return -1 to signify that
        PATH_MAX is not bounded.


glibc has certain extensions to avoid the buffer
overflow. One option is to use
canonicalize_file_name(), another is to specify a NULL
as the second argument to realpath() (which essentially
makes it behave like canonicalize_file_name(). Relevant
documentation:

info libc
http://www.delorie.com/gnu/docs/glibc/libc_279.html

Attached is a patch to use canonicalize_file_name if
available.
msg48754 - (view) Author: Matejcik (spektrum) Date: 2005-09-26 13:21
Logged In: YES 
user_id=631694

there are two bugs in the patch: one is explained on
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=169046
the other one is that with this patch, the value of n
(length to be copied) stays zero all the time, so no copy
occurs and various things happen.

i have a patch, but don't know how to attach it - should i
mail it to you?
msg48755 - (view) Author: Peter Jones (pjones) Date: 2005-09-26 19:11
Logged In: YES 
user_id=27350

There are actually a few more, very subtle problems with the
new patch as well.  I'll get misa to update a patch once
I've worked through them.

In general, regarding Red Hat's bz# 169046, there's no
reason to do readlink() or realpath() if you've got
canonicalize_file_name; it should provide both sets of
functionality just fine.
msg48756 - (view) Author: Peter Jones (pjones) Date: 2005-09-26 19:14
Logged In: YES 
user_id=27350

... but I see you meant the condition different than I
parsed your explanation.  Anyway, I'll get misa to upload a
new patch once I've figured out what's going wrong entirely.
msg48757 - (view) Author: Matejcik (spektrum) Date: 2006-09-01 14:12
Logged In: YES 
user_id=631694

if I may kindly remind you that this is a good time to
implement some kind of a patch ;e) now that python 2.5 is
nearing completion, and there's even a security advisory
(http://nvd.nist.gov/nvd.cfm?cvename=CVE-2006-1542)...

I have a working patch, applicable against 2.5c1, at
http://users.suse.cz/~jmatejek/python-2.4.2-canonicalize2.patch
(note the version number - the affected function didn't
change since 2.4.2)
msg86585 - (view) Author: Daniel Diniz (ajaksu2) * (Python triager) Date: 2009-04-26 01:15
See http://nvd.nist.gov/nvd.cfm?cvename=CVE-2006-1542
msg87090 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2009-05-04 00:24
The patch introduces a memory leak, canonicalize_file_name() returns a 
new allocated string which is not freed later.
msg87446 - (view) Author: Mihai Ibanescu (mibanescu) Date: 2009-05-08 15:32
Disclaimer: this bug is more than 3 years old, I don't remember all the
details.

Victor, solely reading the patch I see:

+#ifdef HAVE_CANONICALIZE_FILE_NAME
+		free(argv0);
+#endif /* HAVE_CANONICALIZE_FILE_NAME */

so argv0 (the string where the results of canonicalize_file_name() is
stored) should be freed.

Is there another branch that does not hit this code, that would create
the memory leak?
msg114627 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-08-22 00:55
Surely this security issue should be addressed?
msg142586 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-20 23:56
The latest POSIX versions (*) allow NULL to be passed for the target memory area, meaning that realpath() will allocate as much memory as necessary by itself. This essentially does the same thing as canonicalize_file_name(), but in a standard way rather than by relying on a GNU extension.

I suppose that possibility could be checked at configure time.

(*) http://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html
msg142664 - (view) Author: Mihai Ibanescu (mibanescu) Date: 2011-08-22 03:29
It's a real shame the original patch was not applied before py3k was branched, the code is now different.

Antoine, my autoconf knowledge is limited, I don't know how you'd test for realpath accepting a NULL argument (and doing the right thing) at compile time.

My involvement with this bug is fairly limited at this point, I would like to see it fixed, but having seen no movement on it for almost 6 years now, maybe it's not as critical as I thought it was.
msg142927 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2011-08-24 21:23
See issue #12801: it has a more recent patch.
msg275217 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2016-09-08 23:49
Victor, #12801 was closed. What about this ticket?
History
Date User Action Args
2022-04-11 14:56:13adminsetstatus: pending -> open
github: 42400
2016-09-08 23:49:28christian.heimessetstatus: open -> pending

messages: + msg275217
2014-02-03 17:48:11Arfreversetnosy: + Arfrever
2014-02-03 17:06:51BreamoreBoysetnosy: - BreamoreBoy
2013-07-05 23:34:31christian.heimessetnosy: + christian.heimes

versions: + Python 3.4
2011-08-24 21:23:59vstinnersetmessages: + msg142927
2011-08-22 03:29:40mibanescusetmessages: + msg142664
2011-08-21 12:34:01eric.araujosetversions: - Python 2.6, Python 3.1
2011-08-20 23:56:58pitrousetversions: + Python 3.3
nosy: + pitrou

messages: + msg142586

stage: test needed -> needs patch
2010-08-22 00:59:53eric.araujosetversions: + Python 2.7, Python 3.2, - Python 3.0
2010-08-22 00:55:12BreamoreBoysetnosy: + BreamoreBoy
messages: + msg114627
2009-06-22 21:03:31matejciksetnosy: + matejcik
2009-05-08 15:32:15mibanescusetnosy: + mibanescu
messages: + msg87446
2009-05-04 00:24:05vstinnersetmessages: + msg87090
2009-04-26 01:15:02ajaksu2setversions: + Python 2.6, Python 3.0, Python 3.1, - Python 2.5
nosy: + ajaksu2, vstinner

messages: + msg86585

stage: test needed
2008-01-12 01:41:18akuchlingsettype: security
2005-09-22 14:54:06misacreate