[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Julian Berman]

Change by Julian Berman :


--
nosy: +Julian

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Steve Dower]

Steve Dower  added the comment:

> I don't think that adding a parameter for opt-in for security is a good 
> approach.

I meant to have it set by default on 3.10, when we do not have to worry 
about breaking users.

If it takes years for users to get to 3.10, we should reevaluate our 
release cycle, not whether we aggressively break maintenance releases.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[STINNER Victor]

STINNER Victor  added the comment:

> In this case, having it off by default goes further to prevent breakage

PyYAML was unsafe by default: it allowed to execute arbitary Python code by 
default. It took years to change the default to "safe". I don't think that 
adding a parameter for opt-in for security is a good approach. An application 
can use ipaddress internally without being aware of using it, if it's done by a 
third party module. It's hard to prevent security vulnerabilities if people 
have to "opt-in" for security.

I prefer to break code and force people to manually get back the old behavior. 
It's better to make 90% safe by default but make 10% of people unhappy.

It's uncommon to pass IPv4 addresses with leading zeros.

If you want to tolerate leading zeros, you don't have to modify the ipaddress 
for that, you can pre-process your inputs: it works on any Python version with 
or without the fix.

>>> def reformat_ip(address): return '.'.join(part.lstrip('0') if part != '0' 
>>> else part for part in address.split('.'))
... 
>>> reformat_ip('0127.0.0.1')
'127.0.0.1'

Or with an explicit loop for readability:

def reformat_ip(address):
parts = []
for part in address.split('.'):
if part != "0":
part = part.lstrip('0')
parts.append(part)
return '.'.join(parts)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Steve Dower]

Steve Dower  added the comment:

The important quote from the linked issue seems to be:

> Our new separator= parameter does not allow one to achieve the previous 
> behavior if mixing and matching & And ; was intended to be allowed, as it is 
> a single separator rather than a set of separators.

So arguably, we added _the wrong_ parameter in that case, because it only 
allowed choosing between behaviours not including the "bad" behaviour. We 
should've added one that was "give me back the previous behaviour".

In this case, having it off by default goes further to prevent breakage, and I 
wouldn't be opposed to a process level opt-in (e.g. a module-level flag), so 
that _applications_ have a way to force their dependencies to use the safer 
behaviour without needing to patch them. Similarly, a process level opt-out 
also seems good enough if we were to have it on by default.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[STINNER Victor]

STINNER Victor  added the comment:

> Withdrawing the readiness - @ambv and I would prefer to see this behind a 
> flag (probably "strict" parsing), on by default for 3.10, and maybe on by 
> default for 3.9/earlier.

Last time we added a new parameter in a stable branch, it didn't go well:
https://bugs.python.org/issue42967#msg387638

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Steve Dower]

Steve Dower  added the comment:

(Copied from my comment on the PR, following the one where I said this was 
ready to go.)

Withdrawing the readiness - @ambv and I would prefer to see this behind a flag 
(probably "strict" parsing), on by default for 3.10, and maybe on by default 
for 3.9/earlier.

The main reasoning being that this isn't our vulnerability, but an 
inconsistency with other vulnerable libraries. The current fix is the best it 
can be, but it doesn't prevent the vulnerability, it just causes Python to 
break first. So it ought to be relatively easy to retain the flexible (though 
admittedly non-sensical) behaviour for those who currently rely on it.

--
nosy: +steve.dower

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Łukasz Langa]

Łukasz Langa  added the comment:

Deferred the blocker to the next regular release due to lack of activity in 
time for the current expedited releases.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Łukasz Langa]

Change by Łukasz Langa :


--
priority: release blocker -> deferred blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Christian Heimes]

Change by Christian Heimes :


--
nosy: +lukasz.langa
priority: critical -> release blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[George-Cristian Bîrzan]

Change by George-Cristian Bîrzan :


--
nosy: +gc2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[STINNER Victor]

STINNER Victor  added the comment:

> The patch should not have landed in 3.8. At a bare minimum the patch should 
> have been postponed until documentation was updated. Since 3.8 the ipaddresss 
> does not behave as documented. A similar security issue in NPM was published 
> two days ago, CVE-2021-28918.

Link: https://sick.codes/sick-2021-011

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Christian Heimes]

Change by Christian Heimes :


--
keywords: +patch
pull_requests: +23844
stage: needs patch -> patch review
pull_request: https://github.com/python/cpython/pull/25099

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Christian Heimes]

Christian Heimes  added the comment:

Serhiy was right, this is a security issue.

The patch should not have landed in 3.8. At a bare minimum the patch should 
have been postponed until documentation was updated. Since 3.8 the ipaddresss 
does not behave as documented. A similar security issue in NPM was published 
two days ago, CVE-2021-28918.

I proposed to not only revert the change, but also tighten the check for 
leading zeros so it behaves like glibc's inet_pton(). It refuses any IPv4 
string with a leading zero.

>>> socket.inet_pton(socket.AF_INET, "01.1.1.1")
Traceback (most recent call last):
  File "", line 1, in 
OSError: illegal IP address string passed to inet_pton
>>> socket.inet_pton(socket.AF_INET, "1.1.1.01")
Traceback (most recent call last):
  File "", line 1, in 
OSError: illegal IP address string passed to inet_pton

--
components: +Library (Lib)
keywords: +3.8regression, 3.9regression -3.2regression
nosy: +christian.heimes
priority: normal -> critical
type: behavior -> security
versions: +Python 3.10, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Nick Coghlan]

Nick Coghlan  added the comment:

The recommended handling in the article that Serhiy mentions is to strip the 
leading zeroes, which the ipaddress module will still do - it's only being made 
more tolerant on input. That means it will become usable as a prefilter step 
(pass string with potentially leading zeroes to ipaddress, get string with no 
leading zeroes out).

So that means the one part we missed is the docs update (together with a 
versionchanged note in the module docs themselves)

--
assignee:  -> docs@python
components: +Documentation -Library (Lib)
nosy: +docs@python

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Eric V. Smith]

Eric V. Smith  added the comment:

I think it should be 3.8 only, and the docs should be updated. Apologies for 
not catching that earlier: I searched via Google, which was a mistake.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

See also the article "Ping and FTP Resolve IP Address with Leading Zero as 
Octal" 
(https://web.archive.org/web/20061206211851/http://support.microsoft.com/kb/115388).

This is still true in Windows 10.

So it is safer to reject IPv4 addresses with leading zeros that can be 
ambiguously interpreted. Otherwise this can create a security hole.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Ned Deily]

Ned Deily  added the comment:

ipaddress is behaving as documented:

"The following constitutes a valid IPv4 address:

A string in decimal-dot notation, consisting of four decimal integers in the 
inclusive range 0–255, separated by dots (e.g. 192.168.0.1). Each integer 
represents an octet (byte) in the address. Leading zeroes are tolerated only 
for values less than 8 (as there is no ambiguity between the decimal and octal 
interpretations of such strings). [...]"

https://docs.python.org/3/library/ipaddress.html

I can sort of understand imposing that restriction in a Python 2 world where 
leading zeros implied octal and Python 3 outright rejects such forms of 
integers to avoid the ambiguity.  That said, there's no particular reason why 
the components of an IPv4 string acceptable to ipaddress *have* to follow the 
same rules so I'm +0 on making the change at all.  It's a bit of a stretch to 
consider it a bug when it appears to be behaving as documented but I would 
expect such a change to fix more problems than causing them so I'm OK if you 
want to backport it.

But, in any case, the documentation for 3.8 and/or 3.7 needs to be updated.

--
keywords: +3.2regression -patch
resolution: fixed -> 
stage: resolved -> needs patch
status: pending -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
status: open -> pending

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
nosy: +ned.deily
status: pending -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Nick Coghlan]

Nick Coghlan  added the comment:

I've merged the change for Python 3.8 (thanks Joel!).

I'm not sure whether to classify it as an enhancement or as an interoperability 
bug fix, though, so I've put the status to pending and added Ned to the nosy 
list to get his thoughts as the Python 3.7 RM.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> pending

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Nick Coghlan]

Nick Coghlan  added the comment:


New changeset e653d4d8e820a7a004ad399530af0135b45db27a by Nick Coghlan (Joel 
Croteau) in branch 'master':
bpo-36384: Remove check for leading zeroes in IPv4 addresses (GH-12577)
https://github.com/python/cpython/commit/e653d4d8e820a7a004ad399530af0135b45db27a


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
nosy: +ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Roundup Robot]

Change by Roundup Robot :


--
keywords: +patch
pull_requests: +12521
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Eric V. Smith]

Eric V. Smith  added the comment:

I agree that this is not a useful check.

--
nosy: +eric.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[SilentGhost]

Change by SilentGhost :


--
nosy: +pmoody
versions:  -Python 2.7, Python 3.5, Python 3.6, Python 3.7, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36384] ipaddress Should not reject IPv4 addresses with leading zeroes as ambiguously octal

[Joel Croteau]

New submission from Joel Croteau :

I understand to a certain extent the logic in not allowing IPv4 octets that 
might ambiguously be octal, but in practice, it just seems like it creates 
additional parsing hassle needlessly. I have never in many years of working on 
many networked systems seen anyone use dotted octal format, it is actually 
specifically forbidden by RFC 3986 
(https://tools.ietf.org/html/rfc3986#section-7.4), and it means that the 
ipaddress module throws exceptions on many perfectly valid IP addresses just 
because they have leading zeroes. Since the module doesn't support dotted octal 
or dotted hex anyway, this check seems a little pointless. If nothing else, 
there should be a way to disable this check by specifying that your IPs are in 
fact dotted decimal, otherwise it seems like it's just making you have to do 
extra parsing work or just write your own implementation.

--
components: Library (Lib)
messages: 338514
nosy: Joel Croteau
priority: normal
severity: normal
status: open
title: ipaddress Should not reject IPv4 addresses with leading zeroes as 
ambiguously octal
type: behavior
versions: Python 2.7, Python 3.5, Python 3.6, Python 3.7, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com