PyType_FromSpecWithBases: spec->name is dereferenced before checking for NULL

@benjaminp

BPO	34501
Nosy	@benjaminp, @berkerpeksag, @serhiy-storchaka, @izbyshev, @enedil, @miss-islington
PRs	bpo-34501: PyType_FromSpecWithBases: Check spec->name before derefere… #8930 [3.7] closes bpo-34501: PyType_FromSpecWithBases: Check spec->name before dereferencing it. (GH-8930) #8931 [3.6] closes bpo-34501: PyType_FromSpecWithBases: Check spec->name before dereferencing it. (GH-8930) #8932

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-08-25.18:53:49.746>
created_at = <Date 2018-08-25.15:26:40.523>
labels = ['interpreter-core', '3.8', 'type-bug', '3.7']
title = 'PyType_FromSpecWithBases: spec->name is dereferenced before checking for NULL'
updated_at = <Date 2018-08-25.19:17:16.347>
user = 'https://github.com/izbyshev'

bugs.python.org fields:

activity = <Date 2018-08-25.19:17:16.347>
actor = 'miss-islington'
assignee = 'none'
closed = True
closed_date = <Date 2018-08-25.18:53:49.746>
closer = 'benjamin.peterson'
components = ['Interpreter Core']
creation = <Date 2018-08-25.15:26:40.523>
creator = 'izbyshev'
dependencies = []
files = []
hgrepos = []
issue_num = 34501
keywords = ['patch']
message_count = 7.0
messages = ['324073', '324088', '324090', '324091', '324092', '324093', '324094']
nosy_count = 6.0
nosy_names = ['benjamin.peterson', 'berker.peksag', 'serhiy.storchaka', 'izbyshev', 'enedil', 'miss-islington']
pr_nums = ['8930', '8931', '8932']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue34501'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

In the following snippet from PyType_FromSpecWithBases() in Objects/typeobject.c, spec->name is dereferenced by strrchr() but then is checked for NULL:

/* Set the type name and qualname */
s = strrchr(spec->name, '.');
if (s == NULL)
    s = (char*)spec->name;
else
    s++;

[snip]

type->tp_name = spec->name;
if (!type->tp_name)
    goto fail;

This was reported by Svace static analyzer.

If I were to check spec->name first, what error should I report to the caller? Is something like the following OK?

    if (spec->name == NULL) {
        PyErr_SetString(PyExc_SystemError,
                       "Type spec does not define the name field.");
        goto fail;
    }

Your suggested fix seems fine to me.

But if the error message is set, then for consistency, it should be also the case that the later checks set some kind of error message.

@enedil

@enedil It seems that all later check either set a message or simply propagate the error set by a called function. Did you mean any specific later check?

New changeset 5f79b50 by Benjamin Peterson (Alexey Izbyshev) in branch 'master':
closes bpo-34501: PyType_FromSpecWithBases: Check spec->name before dereferencing it. (GH-8930)
5f79b50

New changeset 15dadac by Miss Islington (bot) in branch '3.7':
closes bpo-34501: PyType_FromSpecWithBases: Check spec->name before dereferencing it. (GH-8930)
15dadac

New changeset 323a91b by Miss Islington (bot) in branch '3.6':
closes bpo-34501: PyType_FromSpecWithBases: Check spec->name before dereferencing it. (GH-8930)
323a91b

Navigation Menu

Repository navigation

PyType_FromSpecWithBases: spec->name is dereferenced before checking for NULL #78682

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

PyType_FromSpecWithBases: spec->name is dereferenced before checking for NULL #78682

Description

Activity

izbyshev commented on Aug 25, 2018

benjaminp commented on Aug 25, 2018

enedil commented on Aug 25, 2018

izbyshev commented on Aug 25, 2018

benjaminp commented on Aug 25, 2018

miss-islington commented on Aug 25, 2018

miss-islington commented on Aug 25, 2018

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions