[issue14036] urlparse insufficient port property validation
Martin Panter added the comment: See Issue 20059 proposing to change this to raise ValueError -- nosy: +vadmium ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 988903cf24c5 by Senthil Kumaran in branch '2.7': Issue #14036: return None when port in urlparse cross 65535 http://hg.python.org/cpython/rev/988903cf24c5 New changeset d769e64aed79 by Senthil Kumaran in branch '3.2': Issue #14036: return None when port in urlparse cross 65535 http://hg.python.org/cpython/rev/d769e64aed79 New changeset b4d257c64db7 by Senthil Kumaran in branch 'default': Issue #14036: return None when port in urlparse cross 65535 http://hg.python.org/cpython/rev/b4d257c64db7 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
Senthil Kumaran sent...@uthcode.com added the comment: This is taken care. I was not really convinced on the need as likely seemed a non issue from urlparse standpoint, But still there is no harm in returning valid port as semantically the attribute stands for a port. Thanks! -- assignee: - orsenthil resolution: - fixed stage: - committed/rejected status: open - closed type: enhancement - behavior ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
Senthil Kumaran sent...@uthcode.com added the comment: I am not sure if anything should be done to this request. Saying that int(99,10) is converting to 99L in Python2.7 it is a bug/security issue is hypothetical. Practically, such high port numbers cannot exist. I suggest, we close this issue as won't fix. Ezio, I noticed that you changed from pending to open. If you had any ideas for this issue, please share, otherwise you can consider closing this too. Thanks! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
zulla d...@defendassist.com added the comment: Your comment is completely senseless, sorry. Of course such high port numbers do not exist. An attacker is counting on that. Imagine something like that pass_to_cython(urlparse(http://google.de:99**99[to be calculated]).port) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
zulla d...@defendassist.com added the comment: we should at least check if the .port attribute is an intereger = 1 and = 65535. _because_ this is the only valid port range. otherwise, it is no valid port. but it may be a integer overflow attack attempt when a developer uses .port, he is counting on the result being valid -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
Senthil Kumaran sent...@uthcode.com added the comment: pass_to_cython(urlparse(http://google.de:99**99[to be calculated]).port) is no different than sending pass_to_cython(99**99[to be calculated]) In that case, would the former make a security loop hole in urlparse? Looks pretty contrived to me as an example for .port bug. However, I agree with one point in your assertion, namely that port be checked that it is within the range integer = 1 and = 65535. If it is not, return None as a response in port. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
Ezio Melotti ezio.melo...@gmail.com added the comment: Ezio, I noticed that you changed from pending to open. That was an accident, I just meant to add my self to the nosy. I didn't have time yet to read all the messages and comment on the issue. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
Changes by Ezio Melotti ezio.melo...@gmail.com: -- nosy: +ezio.melotti status: pending - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
Senthil Kumaran sent...@uthcode.com added the comment:
Couple of points:
1. On your last example, which webserver treats 'L' as part of port number? I
can't of anything.
2. Can you write a real application which is listening to beyond 65535? Which
platform would it be?
Current way of handling invalid port like, int('foo') by raising ValueError
seems to be a better than returning a None. A better error message could be
desirable, but that does not make it a security issue.
Additionally, for the example of int changing long integer to 'L' appended one
would a 2.x case as it is no longer the behavior in 3.x
Also, I would advice to look at getPort function in a C library or a Java
library and see what it does. The only difference I see is, they return -1
where Python returns None.
I am changing the request type to an enhancement, because there is not a valid
argument to support that it is a security issue.
--
status: open - pending
type: security - enhancement
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14036
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
zulla d...@defendassist.com added the comment: u(http://www.google.com:99;).port 99L -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
Nick Coghlan ncogh...@gmail.com added the comment: Could you provide some failing examples? The suggestion also seems to run slightly at odds with itself - in one part, silently replacing an invalid port specification with a different value, in another adding additional validation checks. Also, rather than hardcoding default port numbers for selected protocols, it would make more sense to just look them up via socket.getservbyname(scheme) (and still return None if the scheme isn't recognised). However, I'll need to think about that one for a while - urlparse is designed to be almost purely a string *parsing* library. Looking up default port numbers if one isn't specified really isn't its job. (For one thing, you'd break the ability to exactly recreate the original URL text from the parsed version). There should definitely by a try/except around that conversion to int(), though - it's just that I'm not yet sure what an appropriate return value is at that point. The raw port string? None? Should there be a separate raw_port descriptor that always returns some kind of string, with the port descriptor promising to always return a valid 16-bit port number or None? -- nosy: +ncoghlan versions: -Python 3.1, Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
New submission from zulla d...@defendassist.com: The port component of a URL is not properly be sanitized or validated. This may lead to the evasion of netloc/hostname based filters or exceptions. -- components: Library (Lib) files: testurllib.py messages: 153512 nosy: zulla priority: normal severity: normal status: open title: urlparse insufficient port property validation type: security versions: Python 2.7, Python 3.1, Python 3.2, Python 3.3, Python 3.4 Added file: http://bugs.python.org/file24535/testurllib.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
zulla d...@defendassist.com added the comment: The port and netloc component of a ParsedResult-object is not properly sanitized or validated. This may lead to bypass-able hostname-based filters. Remote Crash vulnerabilities be be also possible. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
R. David Murray rdmur...@bitdance.com added the comment: Did you upload urlparse.py to the issue by accident? Can you please provide some examples of where you think the current code is producing incorrect results? -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
zulla d...@defendassist.com added the comment:
Hi. No, it's a patched version. It won't crash under circumstances like that
[1] and won't succeed with invalid input:
import urlparse
urlparse.urlparse(http://www.google.com:foo;)
ParseResult(scheme='http', netloc='www.google.com:foo', path='', params='',
query='', fragment='')
urlparse.urlparse(http://www.google.com:foo;).port
Traceback (most recent call last):
File stdin, line 1, in module
File
/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/urlparse.py,
line 105, in port
port = int(netloc.split(':')[1], 10)
ValueError: invalid literal for int() with base 10: 'foo'
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14036
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
zulla d...@defendassist.com added the comment: Whops. I forgot an int() :-) Here's the right patch. -- Added file: http://bugs.python.org/file24540/testurllib.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
Changes by zulla d...@defendassist.com: Removed file: http://bugs.python.org/file24535/urlparse.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
R. David Murray rdmur...@bitdance.com added the comment: It's not a patch if it is the whole file. A diff would be much more useful, since then we could see the changes easily. This kind of change would require a bit of discussion. I'm doubtful that it would be applied as a bug fix, and we might even want the validation to be optional and not the default. Part of the issue is that urlparse was originally based on the older standards, as you can see from the docstring. You may find others to agree with you, but personally it doesn't look to me like this rises to the level of a security issue. -- nosy: +orsenthil ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14036] urlparse insufficient port property validation
zulla d...@defendassist.com added the comment: I understand your point of view, but I disagree. Various libraries and projects rely on urlparse.urlparse and urllib.parse.urlparse. This bug just blew up in my face. I'm working with Cython and PyQt4. When a developer relies on ParseResult().netloc being a valid netloc, and .port being None [bool(False)] or a integer between 1-65535 really bad things can happen in a environment that has 0-tolerance for security issues (like C/C++ mixed in python). I agree that the if self.scheme == http: return 80 elif self.scheme == https: [...] part of my patch is debetable, but we should _at least_ ensure that IF there is a ParseResult().port, the developer can be sure that it is a valid port between 1-65545. i apologize for upload the whole file; i attached the diff now. regards, dan -- keywords: +patch Added file: http://bugs.python.org/file24541/urlparse.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14036 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
