gh-145376: Fix null pointer deref in md5module.c by eendebakpt · Pull Request #145422 · python/cpython

eendebakpt · 2026-03-02T14:47:38Z

Avoid a null pointer deref in the case of an error path in the constructors (e.g. MD5Type_copy_impl)

Issue found using Claude.

Issue: Various reference leaks in the core #145376

picnixz

I think this pattern is also present in SHA-* and other modules as it's essentially C/C a b it everywhere.

eendebakpt · 2026-03-02T17:31:52Z

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.

cpython/Modules/sha1module.c

Line 89 in 02288bf

if (ptr->hash_state != NULL) {

There the hash_state is also set to NULL, not sure why that is.

picnixz · 2026-03-02T17:43:26Z

C/C a b is

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)

There the hash_state is also set to NULL, not sure why that is.

Likely to prevent a double-free (I don't know if it was me or tiran/gpshead who added this)

picnixz · 2026-03-02T17:44:06Z

Actually it was me in 261633b. I just forgot about the MD5 one I think.

picnixz · 2026-03-02T18:09:47Z

Please add a small NEWS entry.
If you find other issues in MD5, you can include them here as well. Same for SHA-*, BLAKE2 and HMAC.

picnixz · 2026-03-02T18:10:12Z

Oh and please also put the state to NULL just to prevent a possible double-free

Hit the wrong button

…ull_md5module

picnixz · 2026-03-02T20:24:09Z

Modules/hmacmodule.c

            int rc = py_hmac_hinfo_ht_add(table, KEY, value);   \
            if (rc < 0) {                                       \
-                PyMem_Free(value);                              \
+                if (value->refcnt == 0) {                       \


This one is already part of an other PR actually.

Namely: #145321.

I would prefer merging PR gh-145321 first since it's older and more complete.

Agreed. I am waiting for the other PR and will rebase accordingly.

i just merged the above.

vstinner

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.

vstinner · 2026-03-04T22:40:07Z

Modules/hmacmodule.c

            int rc = py_hmac_hinfo_ht_add(table, KEY, value);   \
            if (rc < 0) {                                       \
-                PyMem_Free(value);                              \
+                if (value->refcnt == 0) {                       \


I would prefer merging PR gh-145321 first since it's older and more complete.

picnixz · 2026-03-05T07:09:11Z

I prefer to always include a NEWS entry for crashes even if they are rare.

vstinner

LGTM.

Misc/NEWS.d/next/Library/2026-03-02-19-41-39.gh-issue-145376.OOzSOh.rst

Co-authored-by: Victor Stinner <vstinner@python.org>

Modules/hmacmodule.c

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Misc/NEWS.d/next/Library/2026-03-02-19-41-39.gh-issue-145376.OOzSOh.rst

picnixz · 2026-03-06T19:39:37Z

@eendebakpt Thanks for the PR! Once it's merged, could you do a manual backport for 3.13 and an automatic backport for 3.14 (though I'm not entirely sure about conflicts in md5module.c)

miss-islington-app · 2026-03-06T20:00:34Z

Thanks @eendebakpt for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

…honGH-145422) Fix a possible NULL pointer dereference in `md5module.c` and a double-free in `hmacmodule.c`. Those crashes only occur in error paths taken when the interpreter fails to allocate memory. (cherry picked from commit c1d7768) Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>

bedevere-app · 2026-03-06T20:00:46Z

GH-145610 is a backport of this pull request to the 3.14 branch.

bedevere-app · 2026-03-06T20:14:57Z

GH-145611 is a backport of this pull request to the 3.13 branch.

…-145422) (#145610) gh-145376: Fix crashes in `md5module.c` and `hmacmodule.c` (GH-145422) Fix a possible NULL pointer dereference in `md5module.c` and a double-free in `hmacmodule.c`. Those crashes only occur in error paths taken when the interpreter fails to allocate memory. (cherry picked from commit c1d7768) Co-authored-by: Pieter Eendebak <pieter.eendebak@gmail.com>

* gh-145376: Fix crashes in md5module.c Fix a possible NULL pointer dereference in `md5module.c`. This can only occur in error paths taken when the interpreter fails to allocate memory. (cherry-picked from c1d7768) * 📜🤖 Added by blurb_it. * Update Modules/md5module.c --------- Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>

eendebakpt added 2 commits March 2, 2026 12:02

Fix null pointer deref in md5module.c

d5b9513

pep7

6cc205c

eendebakpt requested review from gpshead and picnixz as code owners March 2, 2026 14:47

bedevere-app bot mentioned this pull request Mar 2, 2026

Various reference leaks in the core #145376

Open

bedevere-app bot added the awaiting review label Mar 2, 2026

picnixz reviewed Mar 2, 2026

View reviewed changes

picnixz previously approved these changes Mar 2, 2026

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Mar 2, 2026

bedevere-app bot added awaiting review and removed awaiting merge labels Mar 2, 2026

eendebakpt and others added 6 commits March 2, 2026 20:37

review comments

330128f

📜🤖 Added by blurb_it.

aae8d22

Merge branch 'null_md5module' of github.com:eendebakpt/cpython into n…

c696867

…ull_md5module

Merge branch 'main' into null_md5module

eafcc2d

Merge branch 'null_md5module' of github.com:eendebakpt/cpython into n…

6b4cbff

…ull_md5module

fix

3aa0033

picnixz reviewed Mar 2, 2026

View reviewed changes

vstinner reviewed Mar 4, 2026

View reviewed changes

Merge branch 'main' into null_md5module

fe78d49

eendebakpt marked this pull request as draft March 5, 2026 09:57

bedevere-app bot removed the awaiting review label Mar 5, 2026

vstinner approved these changes Mar 5, 2026

View reviewed changes

Misc/NEWS.d/next/Library/2026-03-02-19-41-39.gh-issue-145376.OOzSOh.rst Outdated Show resolved Hide resolved

bedevere-app bot added the awaiting merge label Mar 5, 2026

Apply suggestion from @vstinner

4ae295b

Co-authored-by: Victor Stinner <vstinner@python.org>

picnixz reviewed Mar 5, 2026

View reviewed changes

Modules/hmacmodule.c Outdated Show resolved Hide resolved

Apply suggestion from @picnixz

3e79541

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

eendebakpt commented Mar 5, 2026

View reviewed changes

Misc/NEWS.d/next/Library/2026-03-02-19-41-39.gh-issue-145376.OOzSOh.rst Outdated Show resolved Hide resolved

eendebakpt added 2 commits March 5, 2026 12:53

Apply suggestion from @eendebakpt

1fa2e83

Merge branch 'main' into null_md5module

46b2511

eendebakpt marked this pull request as ready for review March 5, 2026 12:22

bedevere-app bot added awaiting review and removed awaiting merge labels Mar 5, 2026

picnixz approved these changes Mar 5, 2026

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Mar 5, 2026

Merge branch 'main' into null_md5module

93f1de5

picnixz enabled auto-merge (squash) March 6, 2026 19:38

picnixz merged commit c1d7768 into python:main Mar 6, 2026
50 checks passed

bedevere-app bot removed the awaiting merge label Mar 6, 2026

eendebakpt added the needs backport to 3.14 bugs and security fixes label Mar 6, 2026

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Mar 6, 2026

eendebakpt mentioned this pull request Mar 6, 2026

[3.13] gh-145376: Fix crashes in md5module.c (GH-145422) #145611

Merged

Uh oh!

Conversation

eendebakpt commented Mar 2, 2026 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

eendebakpt commented Mar 2, 2026

Uh oh!

picnixz commented Mar 2, 2026

Uh oh!

picnixz commented Mar 2, 2026

Uh oh!

picnixz commented Mar 2, 2026

Uh oh!

picnixz commented Mar 2, 2026

Uh oh!

picnixz Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

picnixz Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

vstinner Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

eendebakpt Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

gpshead Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

picnixz commented Mar 5, 2026

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

picnixz commented Mar 6, 2026

Uh oh!

Uh oh!

miss-islington-app bot commented Mar 6, 2026

Uh oh!

bedevere-app bot commented Mar 6, 2026

Uh oh!

bedevere-app bot commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

eendebakpt commented Mar 2, 2026 •

edited by bedevere-app bot

Loading