Repository: oasis-open/cti-python-stix2 (python)

Issue:
version arg is not passed down with the call to parse in STIXObjectProperty.clean
If the version argument is passed in on the original call to parse, it should be passed down also in any recursive calls.  This was not the case here: https://github.com/oasis-open/cti-python-stix2/blob/e3c2a3a57b83da8ce914ba7ea738d2e2813d551e/stix2/properties.py#L685.

Pull request description:
Fix STIX version detection from dicts
Fixes #318 .

In particular, 2.1 SCOs without the spec_version property ought to be correctly detected as 2.1 now.  This is done based on the presence of an ID property, and that their type is in the 2.1 observable type mapping.

The functionality is factored out into a dedicated function.  This allows reuse, including recursive reuse when detecting bundle spec version from the spec versions of its contents.  Also added a unit test suite specifically for this capability.

Note that scanning bundle contents is not currently strictly necessary for determining bundle spec version: if spec_version is present => 2.0, else 2.1.  This is trivially detectable.  I thought maybe the original scan was intended for future-proofing, under the assumption that a 2.1+ bundle would never be allowed to hold SCOs of a version *later* than the bundle spec version.  So I kept it.  That implies that 2.1+ bundle spec version could be computed as the max of the spec versions of its contents, with a 2.1 minimum.  In theory, maybe the correct conclusion is the bundle spec version is not less than the max contained object spec version (so could be a range of versions greater or equal), but we are currently only dealing with two versions, 2.0 and 2.1, and I think some of the bridges are best crossed when we come to them.  (In fact, that may argue for not bothering with a scan at all at this point.)

Patch:
diff --git a/stix2/core.py b/stix2/core.py
index aa9044d..0d1fee5 100644
--- a/stix2/core.py
+++ b/stix2/core.py
@@ -58,6 +58,47 @@ def parse(data, allow_custom=False, version=None):
     return obj
 
 
+def _detect_spec_version(stix_dict):
+    """
+    Given a dict representing a STIX object, try to detect what spec version
+    it is likely to comply with.
+
+    :param stix_dict: A dict with some STIX content.  Must at least have a
+        "type" property.
+    :return: A string in "vXX" format, where "XX" indicates the spec version,
+        e.g. "v20", "v21", etc.
+    """
+
+    obj_type = stix_dict["type"]
+
+    if 'spec_version' in stix_dict:
+        # For STIX 2.0, applies to bundles only.
+        # For STIX 2.1+, applies to SCOs, SDOs, SROs, and markings only.
+        v = 'v' + stix_dict['spec_version'].replace('.', '')
+    elif "id" not in stix_dict:
+        # Only 2.0 SCOs don't have ID properties
+        v = "v20"
+    elif obj_type == 'bundle':
+        # Bundle without a spec_version property: must be 2.1.  But to
+        # future-proof, use max version over all contained SCOs, with 2.1
+        # minimum.
+        v = max(
+            "v21",
+            max(
+                _detect_spec_version(obj) for obj in stix_dict["objects"]
+            ),
+        )
+    elif obj_type in STIX2_OBJ_MAPS["v21"]["observables"]:
+        # Non-bundle object with an ID and without spec_version.  Could be a
+        # 2.1 SCO or 2.0 SDO/SRO/marking.  Check for 2.1 SCO...
+        v = "v21"
+    else:
+        # Not a 2.1 SCO; must be a 2.0 object.
+        v = "v20"
+
+    return v
+
+
 def dict_to_stix2(stix_dict, allow_custom=False, version=None):
     """convert dictionary to full python-stix2 object
 
@@ -93,21 +134,8 @@ def dict_to_stix2(stix_dict, allow_custom=False, version=None):
     if version:
         # If the version argument was passed, override other approaches.
         v = 'v' + version.replace('.', '')
-    elif 'spec_version' in stix_dict:
-        # For STIX 2.0, applies to bundles only.
-        # For STIX 2.1+, applies to SDOs, SROs, and markings only.
-        v = 'v' + stix_dict['spec_version'].replace('.', '')
-    elif stix_dict['type'] == 'bundle':
-        # bundles without spec_version are ambiguous.
-        if any('spec_version' in x for x in stix_dict['objects']):
-            # Only on 2.1 we are allowed to have 'spec_version' in SDOs/SROs.
-            v = 'v21'
-        else:
-            v = 'v' + stix2.DEFAULT_VERSION.replace('.', '')
     else:
-        # The spec says that SDO/SROs without spec_version will default to a
-        # '2.0' representation.
-        v = 'v20'
+        v = _detect_spec_version(stix_dict)
 
     OBJ_MAP = dict(STIX2_OBJ_MAPS[v]['objects'], **STIX2_OBJ_MAPS[v]['observables'])
 
@@ -142,6 +170,10 @@ def parse_observable(data, _valid_refs=None, allow_custom=False, version=None):
 
     """
     obj = _get_dict(data)
+
+    if 'type' not in obj:
+        raise ParseError("Can't parse observable with no 'type' property: %s" % str(obj))
+
     # get deep copy since we are going modify the dict and might
     # modify the original dict as _get_dict() does not return new
     # dict when passed a dict
@@ -153,11 +185,8 @@ def parse_observable(data, _valid_refs=None, allow_custom=False, version=None):
         # If the version argument was passed, override other approaches.
         v = 'v' + version.replace('.', '')
     else:
-        # Use default version (latest) if no version was provided.
-        v = 'v' + stix2.DEFAULT_VERSION.replace('.', '')
+        v = _detect_spec_version(obj)
 
-    if 'type' not in obj:
-        raise ParseError("Can't parse observable with no 'type' property: %s" % str(obj))
     try:
         OBJ_MAP_OBSERVABLE = STIX2_OBJ_MAPS[v]['observables']
         obj_class = OBJ_MAP_OBSERVABLE[obj['type']]


Test patch:
diff --git a/stix2/test/test_spec_version_detect.py b/stix2/test/test_spec_version_detect.py
new file mode 100644
index 0000000..d2a9da8
--- /dev/null
+++ b/stix2/test/test_spec_version_detect.py
@@ -0,0 +1,212 @@
+from __future__ import unicode_literals
+
+import pytest
+
+from stix2.core import _detect_spec_version
+
+
+@pytest.mark.parametrize(
+    "obj_dict, expected_ver", [
+        # STIX 2.0 examples
+        (
+            {
+                "type": "identity",
+                "id": "identity--d7f72e8d-657a-43ec-9324-b3ec67a97486",
+                "created": "1972-05-21T05:33:09.000Z",
+                "modified": "1973-05-28T02:10:54.000Z",
+                "name": "alice",
+                "identity_class": "individual",
+            },
+            "v20",
+        ),
+        (
+            {
+                "type": "relationship",
+                "id": "relationship--63b0f1b7-925e-4795-ac9b-61fb9f235f1a",
+                "created": "1981-08-11T13:48:19.000Z",
+                "modified": "2000-02-16T15:33:15.000Z",
+                "source_ref": "attack-pattern--9391504a-ef29-4a41-a257-5634d9edc391",
+                "target_ref": "identity--ba18dde2-56d3-4a34-aa0b-fc56f5be568f",
+                "relationship_type": "targets",
+            },
+            "v20",
+        ),
+        (
+            {
+                "type": "file",
+                "name": "notes.txt",
+            },
+            "v20",
+        ),
+        (
+            {
+                "type": "marking-definition",
+                "id": "marking-definition--2a13090f-a493-4b70-85fe-fa021d91dcd2",
+                "created": "1998-03-27T19:44:53.000Z",
+                "definition_type": "statement",
+                "definition": {
+                    "statement": "Copyright (c) ACME Corp.",
+                },
+            },
+            "v20",
+        ),
+        (
+            {
+                "type": "bundle",
+                "id": "bundle--8379cb02-8131-47c8-8a7c-9a1f0e0986b1",
+                "spec_version": "2.0",
+                "objects": [
+                    {
+                        "type": "identity",
+                        "id": "identity--d7f72e8d-657a-43ec-9324-b3ec67a97486",
+                        "created": "1972-05-21T05:33:09.000Z",
+                        "modified": "1973-05-28T02:10:54.000Z",
+                        "name": "alice",
+                        "identity_class": "individual",
+                    },
+                    {
+                        "type": "marking-definition",
+                        "id": "marking-definition--2a13090f-a493-4b70-85fe-fa021d91dcd2",
+                        "created": "1998-03-27T19:44:53.000Z",
+                        "definition_type": "statement",
+                        "definition": {
+                            "statement": "Copyright (c) ACME Corp.",
+                        },
+                    },
+                ],
+            },
+            "v20",
+        ),
+        # STIX 2.1 examples
+        (
+            {
+                "type": "identity",
+                "id": "identity--22299b4c-bc38-4485-ad7d-8222f01c58c7",
+                "spec_version": "2.1",
+                "created": "1995-07-24T04:07:48.000Z",
+                "modified": "2001-07-01T09:33:17.000Z",
+                "name": "alice",
+            },
+            "v21",
+        ),
+        (
+            {
+                "type": "relationship",
+                "id": "relationship--0eec232d-e1ea-4f85-8e78-0de6ae9d09f0",
+                "spec_version": "2.1",
+                "created": "1975-04-05T10:47:22.000Z",
+                "modified": "1983-04-25T20:56:00.000Z",
+                "source_ref": "attack-pattern--9391504a-ef29-4a41-a257-5634d9edc391",
+                "target_ref": "identity--ba18dde2-56d3-4a34-aa0b-fc56f5be568f",
+                "relationship_type": "targets",
+            },
+            "v21",
+        ),
+        (
+            {
+                "type": "file",
+                "id": "file--5eef3404-6a94-4db3-9a1a-5684cbea0dfe",
+                "spec_version": "2.1",
+                "name": "notes.txt",
+            },
+            "v21",
+        ),
+        (
+            {
+                "type": "file",
+                "id": "file--5eef3404-6a94-4db3-9a1a-5684cbea0dfe",
+                "name": "notes.txt",
+            },
+            "v21",
+        ),
+        (
+            {
+                "type": "marking-definition",
+                "spec_version": "2.1",
+                "id": "marking-definition--34098fce-860f-48ae-8e50-ebd3cc5e41da",
+                "created": "2017-01-20T00:00:00.000Z",
+                "definition_type": "tlp",
+                "name": "TLP:GREEN",
+                "definition": {
+                    "tlp": "green",
+                },
+            },
+            "v21",
+        ),
+        (
+            {
+                "type": "bundle",
+                "id": "bundle--d5787acd-1ffd-4630-ada3-6857698f6287",
+                "objects": [
+                    {
+                        "type": "identity",
+                        "id": "identity--22299b4c-bc38-4485-ad7d-8222f01c58c7",
+                        "spec_version": "2.1",
+                        "created": "1995-07-24T04:07:48.000Z",
+                        "modified": "2001-07-01T09:33:17.000Z",
+                        "name": "alice",
+                    },
+                    {
+                        "type": "file",
+                        "id": "file--5eef3404-6a94-4db3-9a1a-5684cbea0dfe",
+                        "name": "notes.txt",
+                    },
+                ],
+            },
+            "v21",
+        ),
+        # Mixed spec examples
+        (
+            {
+                "type": "bundle",
+                "id": "bundle--e1a01e29-3432-401a-ab9f-c1082b056605",
+                "objects": [
+                    {
+                        "type": "identity",
+                        "id": "identity--d7f72e8d-657a-43ec-9324-b3ec67a97486",
+                        "created": "1972-05-21T05:33:09.000Z",
+                        "modified": "1973-05-28T02:10:54.000Z",
+                        "name": "alice",
+                        "identity_class": "individual",
+                    },
+                    {
+                        "type": "relationship",
+                        "id": "relationship--63b0f1b7-925e-4795-ac9b-61fb9f235f1a",
+                        "created": "1981-08-11T13:48:19.000Z",
+                        "modified": "2000-02-16T15:33:15.000Z",
+                        "source_ref": "attack-pattern--9391504a-ef29-4a41-a257-5634d9edc391",
+                        "target_ref": "identity--ba18dde2-56d3-4a34-aa0b-fc56f5be568f",
+                        "relationship_type": "targets",
+                    },
+                ],
+            },
+            "v21",
+        ),
+        (
+            {
+                "type": "bundle",
+                "id": "bundle--eecad3d9-bb9a-4263-93f6-1c0ccc984574",
+                "objects": [
+                    {
+                        "type": "identity",
+                        "id": "identity--d7f72e8d-657a-43ec-9324-b3ec67a97486",
+                        "created": "1972-05-21T05:33:09.000Z",
+                        "modified": "1973-05-28T02:10:54.000Z",
+                        "name": "alice",
+                        "identity_class": "individual",
+                    },
+                    {
+                        "type": "file",
+                        "id": "file--5eef3404-6a94-4db3-9a1a-5684cbea0dfe",
+                        "name": "notes.txt",
+                    },
+                ],
+            },
+            "v21",
+        ),
+    ],
+)
+def test_spec_version_detect(obj_dict, expected_ver):
+    detected_ver = _detect_spec_version(obj_dict)
+
+    assert detected_ver == expected_ver