New submission from Adam Olsen:

(thanks go to my partner in crime, jorendorff, for helping flesh this out.)

lookdict calls PyObject_RichCompareBool without using INCREF/DECREF on
the key passed.  It's possible for the comparison to delete the key from
the dict, causing its own argument to be deallocated.  This can lead to
bogus results or a crash.

A custom type with its own __eq__ method will implicitly INCREF the key
when passing it as an argument, which prevents incorrect behaviour from
manifesting.  There are a couple ways around this though, as shown in my

components: Interpreter Core
messages: 57925
nosy: rhamphoryncus
severity: normal
status: open
title: lookdict should INCREF/DECREF startkey around PyObject_RichCompareBool
Added file:

#!/usr/bin/env python

# This first approach causes dict to return the wrong value
# for the key.  The dict doesn't create its own reference
# to startkey, meaning that address may get reused.  We
# take advantage of that to swap in a new value without it
# noticing the key has changed.
class Foo(object):
  def __init__(self, value):
    self.value = value
  def __hash__(self):
    return hash(self.value)
  def __eq__(self, other):
    # self gets packed into a tuple to get passed to us,
    # which INCREFs it and prevents us from reusing the
    # address.  To get around that we move the work into
    # our return value's __nonzero__ method instead.
    return BadBool(self.value, other.value)

class BadBool(object):
  def __init__(self, a, b):
    self.a = a
    self.b = b
  def __nonzero__(self):
    global counter
    if not counter:
      counter = 1
      # This is the bad part.  Conceivably, another thread
      # might do this without any malicious behaviour
      # involved.
      del d[d.keys()[0]]
      d[Foo(2**32+1)] = 'this is never the right answer'
    return self.a == self.b

print "Test 1, using __eq__(a, b).__nonzero__()"
d = {}
counter = 0
d[Foo(2)] = 'this is an acceptable answer'
print d.get(Foo(2), 'so is this')
print '*****'

# This second version uses tuple's tp_richcompare.  tuple
# assumes the caller has a valid reference, but Bar.__eq__
# purges that reference, causing the tuple to be
# deallocated.  Watch only exists to make sure tuple
# continues to use the memory, provoking a crash.
# Interestingly, Watch.__eq__ never gets called.
class Bar(object):
  def __hash__(self):
    return 0
  def __eq__(self, other):
    return True

class Watch(object):
  def __init__(self):
    print "New Watch 0x%x" % id(self)
  def __del__(self):
    print "Deleting Watch 0x%x" % id(self)
  def __hash__(self):
    return 0
  def __eq__(self):
    print "eq Watch", self, other
    return True

print "Test 2, using tuple's tp_richcompare"
d = {}
d[(Bar(), Watch())] = 'hello'
print d[(Bar(), Watch())]
Python-bugs-list mailing list 

Reply via email to