gh-145376: Fix null pointer deref in md5module.c#145422
gh-145376: Fix null pointer deref in md5module.c#145422eendebakpt wants to merge 10 commits intopython:mainfrom
Conversation
picnixz
left a comment
There was a problem hiding this comment.
I think this pattern is also present in SHA-* and other modules as it's essentially C/C a b it everywhere.
Not sure what C/C a b is, but the pattern is indeed the same. E.g. Line 89 in 02288bf There the |
it was meant to be a "C/C a bit" and I used C/C for "carbon copy" (which is essentially to mean "copy-paste" for me)
Likely to prevent a double-free (I don't know if it was me or tiran/gpshead who added this) |
|
Actually it was me in 261633b. I just forgot about the MD5 one I think. |
|
|
Oh and please also put the state to NULL just to prevent a possible double-free |
Modules/hmacmodule.c
Outdated
| int rc = py_hmac_hinfo_ht_add(table, KEY, value); \ | ||
| if (rc < 0) { \ | ||
| PyMem_Free(value); \ | ||
| if (value->refcnt == 0) { \ |
There was a problem hiding this comment.
This one is already part of an other PR actually.
There was a problem hiding this comment.
I would prefer merging PR gh-145321 first since it's older and more complete.
There was a problem hiding this comment.
Agreed. I am waiting for the other PR and will rebase accordingly.
vstinner
left a comment
There was a problem hiding this comment.
The md5 change LGTM. I'm not sure that it's worth it to add a NEWS entry for this change, since it's unlikely to hit this bug in practice.
Modules/hmacmodule.c
Outdated
| int rc = py_hmac_hinfo_ht_add(table, KEY, value); \ | ||
| if (rc < 0) { \ | ||
| PyMem_Free(value); \ | ||
| if (value->refcnt == 0) { \ |
There was a problem hiding this comment.
I would prefer merging PR gh-145321 first since it's older and more complete.
|
I prefer to always include a NEWS entry for crashes even if they are rare. |
Misc/NEWS.d/next/Library/2026-03-02-19-41-39.gh-issue-145376.OOzSOh.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Victor Stinner <vstinner@python.org>
Avoid a null pointer deref in the case of an error path in the constructors (e.g.
MD5Type_copy_impl)Issue found using Claude.