Skip to content

Potential NULL dereference of kw_defaults in has_kwonlydefaults #135302

Closed
@rialbat

Description

@rialbat

The pointer s->v.AsyncFunctionDef.args->kw_defaults is explicitly checked for NULL:

cpython/Python/symtable.c

Lines 2193 to 2195 in aaad2e8

if (s->v.AsyncFunctionDef.args->kw_defaults)
VISIT_SEQ_WITH_NULL(st, expr,
s->v.AsyncFunctionDef.args->kw_defaults);

However, a few lines later, the same pointer is passed unconditionally to the has_kwonlydefaults function:

cpython/Python/symtable.c

Lines 2203 to 2204 in aaad2e8

has_kwonlydefaults(s->v.AsyncFunctionDef.args->kwonlyargs,
s->v.AsyncFunctionDef.args->kw_defaults),

Inside has_kwonlydefaults, the kw_defaults parameter is not checked for NULL before being dereferenced:

cpython/Python/symtable.c

Lines 1783 to 1787 in aaad2e8

for (int i = 0; i < asdl_seq_LEN(kwonlyargs); i++) {
expr_ty default_ = asdl_seq_GET(kw_defaults, i);
if (default_) {
return 1;
}

Linked PRs

Activity

added a commit that references this issue on Jun 9, 2025

pythongh-135302: Fix NULL pointer dereference in has_kwonlydefaults f…

JelleZijlstra

JelleZijlstra commented on Jun 9, 2025

@JelleZijlstra
Member

This isn't necessary because we access kw_defaults only if kwonlyargs is nonempty.

added
interpreter-core(Objects, Python, Grammar, and Parser dirs)
pendingThe issue will be closed if no feedback is provided
on Jun 9, 2025
picnixz

picnixz commented on Jun 9, 2025

@picnixz
Member

@JelleZijlstra I suggested adding an assertion #135303 (comment) but I don't know if you wouldn't prefer a PR that adds assertions also elsewhere rather than just modifiying this single place.

JelleZijlstra

JelleZijlstra commented on Jun 9, 2025

@JelleZijlstra
Member

I don't really have a preference. The code is fine as is; an assertion would be harmless but not help much. Feel free to merge a PR adding assertions if you feel it's useful.

picnixz

picnixz commented on Jun 9, 2025

@picnixz
Member

I think it's only useful if there is a chance to call the function by mistake. If it's not the case, then let's keep the code as is (I don't know if there are other places that would benefit having an assertion)

rialbat

rialbat commented on Jun 10, 2025

@rialbat
ContributorAuthor

This isn't necessary because we access kw_defaults only if kwonlyargs is nonempty.

Got it. Thanks for your answer. I'll close the issue and PR.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    interpreter-core(Objects, Python, Grammar, and Parser dirs)pendingThe issue will be closed if no feedback is provided

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Potential NULL dereference of kw_defaults in has_kwonlydefaults · Issue #135302 · python/cpython

      Follow Lee on X/Twitter - Father, Husband, Serial builder creating AI, crypto, games & web tools. We are friends :) AI Will Come To Life!

      Check out: eBank.nz (Art Generator) | Netwrck.com (AI Tools) | Text-Generator.io (AI API) | BitBank.nz (Crypto AI) | ReadingTime (Kids Reading) | RewordGame | BigMultiplayerChess | WebFiddle | How.nz | Helix AI Assistant