Re: [Python-Dev] Should we fix these errors?

[Martin Panter]

FYI there is also a bug tracker report about this:
https://bugs.python.org/issue27587

On 23 July 2016 at 13:22, Christian Heimes  wrote:
> On 2016-07-22 16:36, Guido van Rossum wrote:
>> Somebody did some research and found some bugs in CPython (IIUC). The
>> published some questionable fragments. If there's a volunteer we could
>> probably easily fix these. (I know we already have occasional Coverity
>> scans and there are other tools too (anybody try lgtm yet?) But this
>> seems honest research (also Python leaves Ruby in the dust :-):
>>
>> http://www.viva64.com/en/b/0414/
>
> I had a closer look at the report. About half of the bugs, maybe more
> are not in the C code of CPython but in OpenSSL code. I really mean
> OpenSSL code, not _ssl.c and _hashopenssl.c. It's safe to assume that
> they forgot to exclude external dependencies.
>
> The issues in ASN1_PRINTABLE_type() [N2], BN_mask_bits() [N4 bn_lib.c,
> digest.c, evp_enc.c], dh_cms_set_peerkey() [N5, dh_ameth.c] and
> cms_env_set_version() [N6, cms_env.c] are all OpenSSL issues and should
> be reported to OpenSSL.
>
> Guido, did the company contact you or do you have Pavel Belikov's email
> address?

Perhaps you can contact him via the email address at
.
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Should we fix these errors?

[Christian Heimes]

On 2016-07-22 16:36, Guido van Rossum wrote:
> Somebody did some research and found some bugs in CPython (IIUC). The
> published some questionable fragments. If there's a volunteer we could
> probably easily fix these. (I know we already have occasional Coverity
> scans and there are other tools too (anybody try lgtm yet?) But this
> seems honest research (also Python leaves Ruby in the dust :-):
> 
> http://www.viva64.com/en/b/0414/

I had a closer look at the report. About half of the bugs, maybe more
are not in the C code of CPython but in OpenSSL code. I really mean
OpenSSL code, not _ssl.c and _hashopenssl.c. It's safe to assume that
they forgot to exclude external dependencies.

The issues in ASN1_PRINTABLE_type() [N2], BN_mask_bits() [N4 bn_lib.c,
digest.c, evp_enc.c], dh_cms_set_peerkey() [N5, dh_ameth.c] and
cms_env_set_version() [N6, cms_env.c] are all OpenSSL issues and should
be reported to OpenSSL.

Guido, did the company contact you or do you have Pavel Belikov's email
address?

Christian

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Should we fix these errors?

[Christian Heimes]

On 2016-07-22 17:31, Chris Angelico wrote:
> On Sat, Jul 23, 2016 at 12:36 AM, Guido van Rossum  wrote:
>> Somebody did some research and found some bugs in CPython (IIUC). The
>> published some questionable fragments. If there's a volunteer we could
>> probably easily fix these. (I know we already have occasional Coverity
>> scans and there are other tools too (anybody try lgtm yet?) But this
>> seems honest research (also Python leaves Ruby in the dust :-):
>>
>> http://www.viva64.com/en/b/0414/
> 
> First and foremost: All of these purported bugs appear to have been
> found by compiling on Windows. Does Coverity test a Windows build? If
> not, can we get it to? These look like the exact types of errors that
> Coverity *would* discover.

No, it doesn't. The Coverity Scan builds only run on X86_64 Linux
platforms. When I took over Coverity Scan for CPython many years ago it
was not possible to support multiple platforms and target with the free
edition. I never tried to upload builds from different platforms because
I feared that it might play havoc with the scan history. Should I check
with Coverity again?

Some of these issues have been found by Coverity and I even have patches
for them, e.g. N6 is CID#1299595. I have 13 patches that I haven't
published and merged yet. None of the issues is critical, though. Since
I forgot how to use hg I have been waiting for the github migration.

Christian
From f84cfa464e4b7d03776afabe9c0819d491c5617b Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Fri, 19 Feb 2016 16:22:23 +0100
Subject: [PATCH 04/13] Fix dereferencing before NULL check in
 _PyState_AddModule()

_PyState_AddModule() accesses a member of PyModuleDef* def first and
then check def for NULL. The other way around is right.

CID 1299595
---
 Python/pystate.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Python/pystate.c b/Python/pystate.c
index ba4dd4c2b5f37ca20f8b9c5afb30053b074f2e50..3fe8ff486ed6838baa92597060af8bcc0ca7e356 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -283,14 +283,15 @@ int
 _PyState_AddModule(PyObject* module, struct PyModuleDef* def)
 {
 PyInterpreterState *state;
-if (def->m_slots) {
-PyErr_SetString(PyExc_SystemError,
-"PyState_AddModule called on module with slots");
-return -1;
-}
-state = GET_INTERP_STATE();
+
 if (!def)
 return -1;
+if (def->m_slots) {
+PyErr_SetString(PyExc_SystemError,
+"PyState_AddModule called on module with slots");
+return -1;
+}
+state = GET_INTERP_STATE();
 if (!state->modules_by_index) {
 state->modules_by_index = PyList_New(0);
 if (!state->modules_by_index)
-- 
2.7.4

___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Should we fix these errors?

[Victor Stinner]

2016-07-22 18:21 GMT+02:00 Random832 :
>> I just fixed it:
>> https://hg.python.org/cpython/rev/6c11f52ab9db
>
> Does INVALID_SOCKET exist on non-windows systems?

Yes, it was already used in almost all places.

When I read again the code, in fact I found other places with "fd < 0"
or "fd = -1". I fixed more code in a second change:
https://hg.python.org/cpython/rev/025281485318

Victor
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Should we fix these errors?

[Random832]

On Fri, Jul 22, 2016, at 11:35, Victor Stinner wrote:
> Oh, the first one is a regression that I introduced in the
> implementation of the PEP 475 (retry syscall on EINTR). I don't think
> that it can be triggered in practice, because socket handles on
> Windows are small numbers, so unlikely to be seen as negative.

The problem as I understand it isn't that handles will be seen as
negative, the problem is that the error return will be seen as
*non*-negative.

> I just fixed it:
> https://hg.python.org/cpython/rev/6c11f52ab9db

Does INVALID_SOCKET exist on non-windows systems? (It's probably safe to
compare against -1, the relevant functions are defined in POSIX as
returning -1 rather than an unspecified negative value)
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Should we fix these errors?

[Victor Stinner]

Oh, the first one is a regression that I introduced in the
implementation of the PEP 475 (retry syscall on EINTR). I don't think
that it can be triggered in practice, because socket handles on
Windows are small numbers, so unlikely to be seen as negative.

I just fixed it:
https://hg.python.org/cpython/rev/6c11f52ab9db

Victor

2016-07-22 16:36 GMT+02:00 Guido van Rossum :
> Somebody did some research and found some bugs in CPython (IIUC). The
> published some questionable fragments. If there's a volunteer we could
> probably easily fix these. (I know we already have occasional Coverity
> scans and there are other tools too (anybody try lgtm yet?) But this
> seems honest research (also Python leaves Ruby in the dust :-):
>
> http://www.viva64.com/en/b/0414/
>
> --
> --Guido van Rossum (python.org/~guido)
> ___
> Python-Dev mailing list
> Python-Dev@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe: 
> https://mail.python.org/mailman/options/python-dev/victor.stinner%40gmail.com
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Should we fix these errors?

[Chris Angelico]

On Sat, Jul 23, 2016 at 12:36 AM, Guido van Rossum  wrote:
> Somebody did some research and found some bugs in CPython (IIUC). The
> published some questionable fragments. If there's a volunteer we could
> probably easily fix these. (I know we already have occasional Coverity
> scans and there are other tools too (anybody try lgtm yet?) But this
> seems honest research (also Python leaves Ruby in the dust :-):
>
> http://www.viva64.com/en/b/0414/

First and foremost: All of these purported bugs appear to have been
found by compiling on Windows. Does Coverity test a Windows build? If
not, can we get it to? These look like the exact types of errors that
Coverity *would* discover.

Fragment N1 is accurate in current Python. (Although the wording of
the report leaves something to be desired. "The SOCKET type in Windows
is unsigned, so comparing it against null is meaningless." - only "x <
0" (not null) is meaningless.) It's lines 1702 and 2026 in current
Python. What's the best solution? Create a macro VALID_SOCKET with two
different definitions, one using "x < 0" and the other using "x !=
INVALID_SOCKET"?

Fragment N2 doesn't appear to be in CPython 3.6 though. I can't find a
file called a_print.c, nor anything with ASN1_PRINTABLE_type in it.
Third party code? 2.7 only? I've no idea.

(It'd be so much more helpful if file paths had been given instead of
just fragment codes. The error messages include file names without
paths in them.)

Fragment N3: Looks like a legit issue.
http://bugs.python.org/issue27591 created with patch.

Fragment N4, N5, N6a: Can't find bn_lib.c, dh_ameth.c, or cms_env.c in
the cpython tree anywhere. Google suggests that they could be part of
OpenSSL (which could be true of a_print.c from N2). Does Python bundle
any OpenSSL source anywhere?

Fragment N6b (there's a completely unrelated issue paired up in N6): I
don't understand all of what's being said here. The error message
quoted refers to _elementtree.c:917, which is an understandable false
positive for the static checker; the problem can't happen, though,
because line 913 checks for NULL and will construct a new empty list,
and line 916 iterates up to the new list's length, so line 917 can
never be reached if self->extra is NULL. But their analyzer can't know
that. On the other hand, the paragraph and code snippet are referring
to _PyState_AddModule in Modules/pystate.c, which is never called with
def=NULL anywhere else in CPython; unless it's intended to be public,
the check on line 292 could simply be removed.

Conclusion: CPython may need some better static checking in Windows
mode, but probably not desperately enough to buy their product (which
is presumably the point of that blog).

ChrisA
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

[Python-Dev] Should we fix these errors?

[Guido van Rossum]

Somebody did some research and found some bugs in CPython (IIUC). The
published some questionable fragments. If there's a volunteer we could
probably easily fix these. (I know we already have occasional Coverity
scans and there are other tools too (anybody try lgtm yet?) But this
seems honest research (also Python leaves Ruby in the dust :-):

http://www.viva64.com/en/b/0414/

-- 
--Guido van Rossum (python.org/~guido)
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com