[Python-Dev] remaining issues from Klocwork static analysis (original) (raw)

"Martin v. Löwis" martin at v.loewis.de
Tue Jul 25 09:40:44 CEST 2006

Neal Norwitz wrote:

# 74 Object/funcobject.c:143 Suspicious deref of ptr before NULL check

Not quite sure what it is complaining about, but

    else if (PyTuple_Check(closure)) {
            Py_XINCREF(closure);
    }

looks indeed suspicious: Why do we check for NULL (XINCREF) when we know closure can't be NULL (Tuple_Check). Drop the X, and see if the warning goes away

#169 Modules/threadmodule.c:497 Memory Leak

Does it say what memory is leaking? Perhaps it complains about boot not being released if ident is not -1, however, in that case, t_bootstrap will release the memory.

# 28 Modules/sre.c:987 Array Index Out of Bounds

Buffer overflow, array index of 'mark' may be outside the bounds. Array 'mark' of size 200 declared at sre.h:77 may use index values 0..536870911. Also there are 3 similar errors on lines 1006, 1225, 1237. (Try limiting mark on line 589?)

ISTM that SRE has a limit of 100 MARK opcodes, meaning a maximum of 100 groups per expression (so you need 200 mark pointers). This can't overrun as sre_compile refuses to compile expressions with more groups.

Of course, a malicious application could craft the opcodes itself (bypassing sre_compile), in which case you could get a buffer overrun.

The right solution is to have a dynamic marks array.

#174 Modules/unicodedata.c:432 Array Index Out of Bounds

Buffer overflow, array index of 'decompprefix' may be outside the bounds. Array 'decompprefix' of size 18 declared at unicodedatadb.h:529 may use index values 18..255. Also there is one similar error on line 433.

This limit is enforced by Tools/unicode/makeunicodedata.py. There are only 18 decomposition prefixes at the moment, yet we use 8 bits for the decomposition prefix (makeunicodedata checks that prefix < 256)

Looking at the code, I now wonder why decomp_data can't be "unsigned short", instead of "unsigned int" (the upper byte is the decomposition length, and it can't be larger than 256, either).

# 36 Modules/cPickle.c:3404 Memory Leak

Memory leak. Dynamic memory stored in 's' allocated through function 'pystrndup' at line 3384 is lost at line 3404. s should not be freed on line 3407, but earlier. PDATAPUSH can return on error and s will not be freed.

Correct. We should not use macros with embedded return statements.

# 61 Modules/sqlite/cursor.c:599 Null pointer may be dereferenced

Null pointer 'self->statement' that comes from line 674 may be dereferenced by passing argument 1 to function 'statementmarkdirty' at line 599.

Looks like a problem. Maybe a break is missing after line 674?

Regards, Martin

More information about the Python-Dev mailing list