Skip to content

gh-145376: Fix null pointer deref in md5module.c#145422

Open
eendebakpt wants to merge 8 commits intopython:mainfrom
eendebakpt:null_md5module
Open

gh-145376: Fix null pointer deref in md5module.c#145422
eendebakpt wants to merge 8 commits intopython:mainfrom
eendebakpt:null_md5module

Conversation

@eendebakpt
Copy link
Contributor

@eendebakpt eendebakpt commented Mar 2, 2026

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

Issue found using Claude.

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@eendebakpt
Copy link
Contributor Author

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.

if (ptr->hash_state != NULL) {

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

@picnixz
Copy link
Member

picnixz commented Mar 2, 2026

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
Copy link
Member

picnixz commented Mar 2, 2026

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

picnixz
picnixz previously approved these changes Mar 2, 2026
@picnixz
Copy link
Member

picnixz commented Mar 2, 2026

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

@picnixz
Copy link
Member

picnixz commented Mar 2, 2026

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

@picnixz picnixz dismissed their stale review March 2, 2026 18:10

Hit the wrong button

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is already part of an other PR actually.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Namely: #145321.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants