Skip to content

Incorrect use of PyObject_IsInstance #68445

Closed
Closed
Incorrect use of PyObject_IsInstance#68445
Assignees
serhiy-storchaka
Labels
extension-modulesC modules in the Modules dirinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-crashA hard crash of the interpreter, possibly with a core dump

Description

@serhiy-storchaka
serhiy-storchaka
opened on May 21, 2015
BPO 24257
Nosy @rhettinger, @bitdancer, @serhiy-storchaka
Files
  • typecheck.patch
  • typecheck_2.patch
    		•

    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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2015-05-22.08:24:25.805>
created_at = <Date 2015-05-21.08:40:46.663>
labels = ['extension-modules', 'interpreter-core', 'type-crash']
title = 'Incorrect use of PyObject_IsInstance'
updated_at = <Date 2015-05-22.08:24:25.804>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2015-05-22.08:24:25.804>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-05-22.08:24:25.805>
closer = 'serhiy.storchaka'
components = ['Extension Modules', 'Interpreter Core']
creation = <Date 2015-05-21.08:40:46.663>
creator = 'serhiy.storchaka'
dependencies = []
files = ['39450', '39453']
hgrepos = []
issue_num = 24257
keywords = ['patch']
message_count = 6.0
messages = ['243739', '243752', '243754', '243756', '243797', '243820']
nosy_count = 4.0
nosy_names = ['rhettinger', 'r.david.murray', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue24257'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    Activity

    1. serhiy-storchaka commented on May 21, 2015

      @serhiy-storchaka
      serhiy-storchaka
      on May 21, 2015
      MemberAuthor

      PyObject_IsInstance() is used incorrectly for testing if Python object is an instance of specified builtin type before direct access to internals of object. This is not correct, because PyObject_IsInstance() checks the __class__ attribute that can be modified and even can be dynamic property. Correct way is to check static type. Proposed patch replaces PyObject_IsInstance() with PyObject_TypeCheck() if this is appropriate.

      See also similar issues bpo-24102 and bpo-24091.

    2. serhiy-storchaka
      added
      extension-modulesC modules in the Modules dir
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      type-crashA hard crash of the interpreter, possibly with a core dump
      on May 21, 2015

    3. bitdancer commented on May 21, 2015

      @bitdancer
      bitdancer
      on May 21, 2015
      Member

      Is there any chance that these changes will break working code, or is it the case that if the current check passes incorrectly one will always get a segfauilt or other error?

    4. rhettinger commented on May 21, 2015

      @rhettinger
      rhettinger
      on May 21, 2015
      Contributor

      is it the case that if the current check passes incorrectly
      one will always get a segfauilt or other error?

      Yes, that is the case. All four of these checks precede a reference to an structure member that depends on being an exact type or subtype. So, yes they are all necessary to prevent segfaults or other undefined behavior.

    5. serhiy-storchaka commented on May 21, 2015

      @serhiy-storchaka
      serhiy-storchaka
      on May 21, 2015
      MemberAuthor

      Yes, it is the case that if the current check passes incorrectly one will always get a segfauilt or other error.

      Added tests for types.SimpleNamespace and sqlite3.Cursor. It is not easy to reproduce a bug for StopIterator (not sure it is reproducible), but the code looks definitely erroneous in any case.

    6. rhettinger commented on May 22, 2015

      @rhettinger
      rhettinger
      on May 22, 2015
      Contributor

      Serhiy, go ahead and apply your patch. The existing code is clearly wrong.

    7. rhettinger
      assigned
      serhiy-storchaka
      on May 22, 2015

    8. python-dev commented on May 22, 2015

      @python-dev
      python-devmannequin
      on May 22, 2015
      Mannequin

      New changeset bccaba8a5482 by Serhiy Storchaka in branch '2.7':
      Issue bpo-24257: Fixed segmentation fault in sqlite3.Row constructor with faked
      https://hg.python.org/cpython/rev/bccaba8a5482

      New changeset c7b9645a6f35 by Serhiy Storchaka in branch '3.4':
      Issue bpo-24257: Fixed incorrect uses of PyObject_IsInstance().
      https://hg.python.org/cpython/rev/c7b9645a6f35

      New changeset a5101529a8a9 by Serhiy Storchaka in branch 'default':
      Issue bpo-24257: Fixed incorrect uses of PyObject_IsInstance().
      https://hg.python.org/cpython/rev/a5101529a8a9

    9. serhiy-storchaka
      closed this as completedon May 22, 2015
    10. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    extension-modulesC modules in the Modules dirinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-crashA hard crash of the interpreter, possibly with a core dump

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions