Repository: oasis-open/cti-python-stix2

Issue:
Infinite recursion on _STIXBase object deserialization
Hi!

There seems to be a bug in the way the `__getitem__`/`__getattr__` functions are implemented in `_STIXBase` (_cti-python-stix2/stix2/base.py_). The bug can be reproduced by trying to serialize/deserialize an object instance, resulting in infinite recursion causing the program to terminate with a `RecursionError` once the max recursion depth is exceeded. 

Here is a short example with copy.copy() triggering the recursion:
```
import copy, stix2
indicator = stix2.Indicator(name="a", labels=["a"], pattern="[file:name = 'a']")
copy.copy(indicator)
```

I believe this is because the unpickling tries to find the optional `__set_state__` method in `_STIXBase`. But since it's not there, the `__getattr__` is being called for it. However, the way `__getattr__` is implemented in the class, the `__getitem__` is called which  tries to look up an item in `self._inner`. Due to the way the deserialization works in general, the object itself is empty and the `__init__` was never called - meaning that the `_inner` attribute was never set. The access attempt on the non-existent `_inner` however triggers a call to `__getattr__` again, resulting in the recursion.

Here are two possible patches, I'm not sure though if that's the proper way of fixing it:

1) Checking for `_inner` access in `__getitem__`:
```
def __getitem__(self, key):
    if key == '_inner':
        raise KeyError
    return self._inner[key]

```
2) Checking for protected attributes in `__getattr__`:
```
def __getattr__(self, name):
    if name.startswith('_'):
        raise AttributeError(name)
    if name in self:
        return self.__getitem__(name)
    raise AttributeError("'%s' object has no attribute '%s'" %
                         (self.__class__.__name__, name))
```

Thanks!

Discussion:

Patch:
diff --git a/stix2/base.py b/stix2/base.py
index 2afba16..9b5308f 100644
--- a/stix2/base.py
+++ b/stix2/base.py
@@ -185,7 +185,13 @@ class _STIXBase(collections.Mapping):
 
     # Handle attribute access just like key access
     def __getattr__(self, name):
-        if name in self:
+        # Pickle-proofing: pickle invokes this on uninitialized instances (i.e.
+        # __init__ has not run).  So no "self" attributes are set yet.  The
+        # usual behavior of this method reads an __init__-assigned attribute,
+        # which would cause infinite recursion.  So this check disables all
+        # attribute reads until the instance has been properly initialized.
+        unpickling = "_inner" not in self.__dict__
+        if not unpickling and name in self:
             return self.__getitem__(name)
         raise AttributeError("'%s' object has no attribute '%s'" %
                              (self.__class__.__name__, name))


Test patch:
diff --git a/stix2/test/test_pickle.py b/stix2/test/test_pickle.py
new file mode 100644
index 0000000..9e2cc9a
--- /dev/null
+++ b/stix2/test/test_pickle.py
@@ -0,0 +1,17 @@
+import pickle
+
+import stix2
+
+
+def test_pickling():
+    """
+    Ensure a pickle/unpickle cycle works okay.
+    """
+    identity = stix2.Identity(
+        id="identity--d66cb89d-5228-4983-958c-fa84ef75c88c",
+        name="alice",
+        description="this is a pickle test",
+        identity_class="some_class"
+    )
+
+    pickle.loads(pickle.dumps(identity))