[Python/upb] Fixed SEGV when attempting to delete a message attribute

haberman · copybara-github · commit de52944f389c · 2023-12-11T16:57:49.000-08:00
Deleting an attribute is not allowed in any Proto Python implementation, but upb was not checking for this case.

PiperOrigin-RevId: 589995449
diff --git a/python/google/protobuf/internal/message_test.py b/python/google/protobuf/internal/message_test.py
@@ -1340,6 +1340,17 @@ def testFieldPresence(self):
     self.assertEqual(False, message.optional_bool)
     self.assertEqual(0, message.optional_nested_message.bb)
 
+  def testDel(self):
+    msg = unittest_pb2.TestAllTypes()
+
+    # Fields cannot be deleted.
+    with self.assertRaises(AttributeError):
+      del msg.optional_int32
+    with self.assertRaises(AttributeError):
+      del msg.optional_bool
+    with self.assertRaises(AttributeError):
+      del msg.repeated_nested_message
+
   def testAssignInvalidEnum(self):
     """Assigning an invalid enum number is not allowed in proto2."""
     m = unittest_pb2.TestAllTypes()
diff --git a/python/message.c b/python/message.c
@@ -1005,6 +1005,12 @@ __attribute__((flatten)) static PyObject* PyUpb_Message_GetAttr(
 static int PyUpb_Message_SetAttr(PyObject* _self, PyObject* attr,
                                  PyObject* value) {
   PyUpb_Message* self = (void*)_self;
+
+  if (value == NULL) {
+    PyErr_SetString(PyExc_AttributeError, "Cannot delete field attribute");
+    return -1;
+  }
+
   const upb_FieldDef* field;
   if (!PyUpb_Message_LookupName(self, attr, &field, NULL,
                                 PyExc_AttributeError)) {
