Repository: oasis-open/cti-stix-validator (python)

Issue:
Validation should fail when `*_bin` values aren't base64 encoded
When the `cti-pattern-matcher` library tries to match a pattern that matches on a value such as `payload_bin` on observed data where `payload_bin` contains unencoded text, the library raises an exception because it can't decode it as base64.  However, that same observed data doesn't show up as a validation error when run through the validator.  So it seems like this is something that should be checked by the validator.

Pull request description:
Update to WD02
Close #17.

Patch:
diff --git a/.gitignore b/.gitignore
index 7ba334e..a347cd5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,3 +32,6 @@ Desktop.ini
 
 # External data cache
 cache.sqlite
+
+# Editors
+*.swp
diff --git a/README.rst b/README.rst
index 4c7e2fc..d9d6260 100644
--- a/README.rst
+++ b/README.rst
@@ -141,7 +141,9 @@ Some checks access Internet resources to determine valid values for certain prop
 +--------+-----------------------------+----------------------------------------+
 |  149   | windows-process-priority-\  | windows-process-ext's 'priority'       |
 |        | format                      | follows the correct format             |
-|        |                             |                                        |
++--------+-----------------------------+----------------------------------------+
+|  150   | hash-length                 | keys in 'hashes'-type properties are   |
+|        |                             | not too long                           |
 +--------+-----------------------------+----------------------------------------+
 |   2    | approved-values             | all 2xx checks are run                 |
 +--------+-----------------------------+----------------------------------------+
@@ -224,6 +226,9 @@ Some checks access Internet resources to determine valid values for certain prop
 |  301   | network-traffic-ports       | network-traffic objects contain both   |
 |        |                             | src_port and dst_port                  |
 +--------+-----------------------------+----------------------------------------+
+|  302   | extref-hashes               | external references SHOULD have hashes |
+|        |                             | if they have a url                     |
++--------+-----------------------------+----------------------------------------+
 
 Governance
 ==========
diff --git a/stix2validator/enums.py b/stix2validator/enums.py
index 0f30935..325926e 100644
--- a/stix2validator/enums.py
+++ b/stix2validator/enums.py
@@ -561,7 +561,6 @@ PROPERTIES = {
 OBSERVABLE_PROPERTIES = {
     'artifact': [
         'type',
-        'description',
         'extensions',
         'mime_type',
         'payload_bin',
@@ -570,7 +569,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'autonomous-system': [
         'type',
-        'description',
         'extensions',
         'number',
         'name',
@@ -578,7 +576,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'directory': [
         'type',
-        'description',
         'extensions',
         'path',
         'path_enc',
@@ -589,14 +586,12 @@ OBSERVABLE_PROPERTIES = {
     ],
     'domain-name': [
         'type',
-        'description',
         'extensions',
         'value',
         'resolves_to_refs'
     ],
     'email-addr': [
         'type',
-        'description',
         'extensions',
         'value',
         'display_name',
@@ -604,7 +599,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'email-message': [
         'type',
-        'description',
         'extensions',
         'is_multipart',
         'date',
@@ -623,7 +617,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'file': [
         'type',
-        'description',
         'extensions',
         'hashes',
         'size',
@@ -643,7 +636,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'ipv4-addr': [
         'type',
-        'description',
         'extensions',
         'value',
         'resolves_to_refs',
@@ -651,7 +643,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'ipv6-addr': [
         'type',
-        'description',
         'extensions',
         'value',
         'resolves_to_refs',
@@ -659,19 +650,16 @@ OBSERVABLE_PROPERTIES = {
     ],
     'mac-addr': [
         'type',
-        'description',
         'extensions',
         'value'
     ],
     'mutex': [
         'type',
-        'description',
         'extensions',
         'name'
     ],
     'network-traffic': [
         'type',
-        'description',
         'extensions',
         'start',
         'end',
@@ -693,7 +681,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'process': [
         'type',
-        'description',
         'extensions',
         'is_hidden',
         'pid',
@@ -711,7 +698,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'software': [
         'type',
-        'description',
         'extensions',
         'name',
         'cpe',
@@ -721,13 +707,11 @@ OBSERVABLE_PROPERTIES = {
     ],
     'url': [
         'type',
-        'description',
         'extensions',
         'value'
     ],
     'user-account': [
         'type',
-        'description',
         'extensions',
         'user_id',
         'account_login',
@@ -745,7 +729,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'windows-registry-key': [
         'type',
-        'description',
         'extensions',
         'key',
         'values',
@@ -755,7 +738,6 @@ OBSERVABLE_PROPERTIES = {
     ],
     'x509-certificate': [
         'type',
-        'description',
         'extensions',
         'is_self_signed',
         'hashes',
diff --git a/stix2validator/errors.py b/stix2validator/errors.py
index c2fc3da..21bcffe 100644
--- a/stix2validator/errors.py
+++ b/stix2validator/errors.py
@@ -123,6 +123,8 @@ def pretty_error(error, verbose=False):
         elif error.schema['title'] == 'url':
             msg = re.sub(r"match '.+'$", 'match the format '
                          'of a URL', msg)
+        elif error.schema['title'] == 'binary':
+            msg = re.sub(r"does not.+'$", 'must be a base64-encoded string', msg)
     elif error.validator == 'pattern' and 'observed_data_refs' in error.schema_path:
         msg = "'observed_data_refs' must refer to Observed Data Objects"
     elif error.validator == 'pattern' and 'where_sighted_refs' in error.schema_path:
diff --git a/stix2validator/schemas b/stix2validator/schemas
index fcf4642..0f37633 160000
--- a/stix2validator/schemas
+++ b/stix2validator/schemas
@@ -1,1 +1,1 @@
-Subproject commit fcf464249c475741c98c0ef8511a639fd85c0b6a
+Subproject commit 0f37633b2e542226023dd907ff10c2d858fbf328
diff --git a/stix2validator/scripts/stix2_validator.py b/stix2validator/scripts/stix2_validator.py
index c8143f5..83ccf49 100644
--- a/stix2validator/scripts/stix2_validator.py
+++ b/stix2validator/scripts/stix2_validator.py
@@ -51,6 +51,8 @@ the validator performs, along with the code to use with the --enable or
 |      |     properties-prefix-lax   | source identifier needed in prefix     |
 | 149  | windows-process-priority-   | windows-process-ext's 'priority'       |
 |      |     format                  | follows the correct format             |
+| 150  | hash-length                 | keys in 'hashes'-type properties are   |
+|      |                             | not too long                           |
 |      |                             |                                        |
 |  2   | approved-values             | all 2xx checks are run                 |
 | 201  | marking-definition-type     | marking definitions use a valid        |
@@ -106,6 +108,8 @@ the validator performs, along with the code to use with the --enable or
 |      |                             | Document Information Dictionary keys   |
 | 301  | network-traffic-ports       | network-traffic objects contain both   |
 |      |                             | src_port and dst_port                  |
+| 302  | extref-hashes               | external references SHOULD have hashes |
+|      |                             | if they have a url                     |
 +------+-----------------------------+----------------------------------------+
 """
 
diff --git a/stix2validator/shoulds.py b/stix2validator/shoulds.py
index c7d2d37..7c50d80 100644
--- a/stix2validator/shoulds.py
+++ b/stix2validator/shoulds.py
@@ -1,4 +1,16 @@
 """Recommended (SHOULD) requirement checking functions
+
+To add a new check:
+- in this module:
+    - define a new function
+    - add the function to CHECKS
+    - add the function to list_shoulds()
+- in utils.py:
+    - add the check code and name to CHECK_CODES
+- in scripts/stix2_validator:
+    - add the check code and name to table
+- in README.rst:
+    - add the check code and name to table
 """
 
 import re
@@ -879,6 +891,109 @@ def windows_process_priority_format(instance):
                                 'windows-process-priority-format')
 
 
+@cyber_observable_check
+def hash_length(instance):
+    """Ensure keys in 'hashes'-type properties are no more than 30 characters long.
+    """
+    for key, obj in instance['objects'].items():
+        if 'type' not in obj:
+            continue
+
+        if obj['type'] == 'file':
+            try:
+                hashes = obj['hashes']
+            except KeyError:
+                pass
+            else:
+                for h in hashes:
+                    if (len(h) > 30):
+                        yield JSONError("Object '%s' has a 'hashes' dictionary"
+                                        " with a hash of type '%s', which is "
+                                        "longer than 30 characters."
+                                        % (key, h), instance['id'], 'hash-algo')
+
+            try:
+                ads = obj['extensions']['ntfs-ext']['alternate_data_streams']
+            except KeyError:
+                pass
+            else:
+                for datastream in ads:
+                    if 'hashes' not in datastream:
+                        continue
+                    for h in datastream['hashes']:
+                        if (len(h) > 30):
+                            yield JSONError("Object '%s' has an NTFS extension"
+                                            " with an alternate data stream that has a"
+                                            " 'hashes' dictionary with a hash of type "
+                                            "'%s', which is longer than 30 "
+                                            "characters."
+                                            % (key, h), instance['id'], 'hash-algo')
+
+            try:
+                head_hashes = obj['extensions']['windows-pebinary-ext']['file_header_hashes']
+            except KeyError:
+                pass
+            else:
+                for h in head_hashes:
+                    if (len(h) > 30):
+                        yield JSONError("Object '%s' has a Windows PE Binary "
+                                        "File extension with a file header hash of "
+                                        "'%s', which is longer than 30 "
+                                        "characters."
+                                        % (key, h), instance['id'], 'hash-algo')
+
+            try:
+                hashes = obj['extensions']['windows-pebinary-ext']['optional_header']['hashes']
+            except KeyError:
+                pass
+            else:
+                for h in hashes:
+                    if (len(h) > 30):
+                        yield JSONError("Object '%s' has a Windows PE Binary "
+                                        "File extension with an optional header that "
+                                        "has a hash of '%s', which is longer "
+                                        "than 30 characters."
+                                        % (key, h), instance['id'], 'hash-algo')
+
+            try:
+                sections = obj['extensions']['windows-pebinary-ext']['sections']
+            except KeyError:
+                pass
+            else:
+                for s in sections:
+                    if 'hashes' not in s:
+                        continue
+                    for h in s['hashes']:
+                        if (len(h) > 30):
+                            yield JSONError("Object '%s' has a Windows PE "
+                                            "Binary File extension with a section that"
+                                            " has a hash of '%s', which is "
+                                            "longer than 30 characters."
+                                            % (key, h), instance['id'], 'hash-algo')
+
+        elif obj['type'] == 'artifact' or obj['type'] == 'x509-certificate':
+            try:
+                hashes = obj['hashes']
+            except KeyError:
+                pass
+            else:
+                for h in hashes:
+                    if (len(h) > 30):
+                        yield JSONError("Object '%s' has a 'hashes' dictionary"
+                                        " with a hash of type '%s', which is "
+                                        "longer than 30 characters."
+                                        % (key, h), instance['id'], 'hash-algo')
+
+
+def extref_hashes(instance):
+    if 'external_references' in instance:
+        for extref in instance['external_references']:
+            if 'url' in extref and 'hashes' not in extref:
+                src = extref['source_name'] if 'source_name' in extref else ''
+                return JSONError("External reference '%s' has a URL but no hash."
+                                 % (src), instance['id'], 'extref-hashes')
+
+
 # Mapping of check names to the functions which perform the checks
 CHECKS = {
     'all': [
@@ -892,6 +1007,7 @@ CHECKS = {
         custom_object_extension_prefix_strict,
         custom_observable_properties_prefix_strict,
         windows_process_priority_format,
+        hash_length,
         vocab_marking_definition,
         relationships_strict,
         vocab_attack_motivation,
@@ -916,6 +1032,7 @@ CHECKS = {
         socket_options,
         pdf_doc_info,
         network_traffic_ports,
+        extref_hashes,
     ],
     'format-checks': [
         custom_object_prefix_strict,
@@ -928,6 +1045,7 @@ CHECKS = {
         custom_object_extension_prefix_strict,
         custom_observable_properties_prefix_strict,
         windows_process_priority_format,
+        hash_length,
     ],
     'custom-object-prefix': custom_object_prefix_strict,
     'custom-object-prefix-lax': custom_object_prefix_lax,
@@ -944,6 +1062,7 @@ CHECKS = {
     'custom-observable-properties-prefix': custom_observable_properties_prefix_strict,
     'custom-observable-properties-prefix-lax': custom_observable_properties_prefix_lax,
     'windows-process-priority-format': windows_process_priority_format,
+    'hash-length': hash_length,
     'approved-values': [
         vocab_marking_definition,
         relationships_strict,
@@ -1017,7 +1136,8 @@ CHECKS = {
     'http-request-headers': http_request_headers,
     'socket-options': socket_options,
     'pdf-doc-info': pdf_doc_info,
-    'network-traffic-ports': network_traffic_ports
+    'network-traffic-ports': network_traffic_ports,
+    'extref-hashes': extref_hashes,
 }
 
 
@@ -1081,6 +1201,8 @@ def list_shoulds(options):
                     validator_list.append(CHECKS['custom-observable-properties-prefix-lax'])
                 if 'windows-process-priority-format' not in options.disabled:
                     validator_list.append(CHECKS['windows-process-priority-format'])
+                if 'hash-length' not in options.disabled:
+                    validator_list.append(CHECKS['hash-length'])
 
             if 'approved-values' not in options.disabled:
                 if 'marking-definition-type' not in options.disabled:
@@ -1124,6 +1246,8 @@ def list_shoulds(options):
 
             if 'network-traffic-ports' not in options.disabled:
                 validator_list.append(CHECKS['network-traffic-ports'])
+            if 'extref-hashes' not in options.disabled:
+                validator_list.append(CHECKS['extref-hashes'])
 
     # --enable
     if options.enabled:
diff --git a/stix2validator/util.py b/stix2validator/util.py
index bd01e75..584789d 100644
--- a/stix2validator/util.py
+++ b/stix2validator/util.py
@@ -103,6 +103,7 @@ CHECK_CODES = {
     '147': 'custom-observable-properties-prefix',
     '148': 'custom-observable-properties-prefix-lax',
     '149': 'windows-process-priority-format',
+    '150': 'hash-length',
     '2': 'approved-values',
     '201': 'marking-definition-type',
     '202': 'relationship-types',
@@ -129,7 +130,8 @@ CHECK_CODES = {
     '274': 'http-request-headers',
     '275': 'socket-options',
     '276': 'pdf-doc-info',
-    '301': 'network-traffic-ports'
+    '301': 'network-traffic-ports',
+    '302': 'extref-hashes',
 }
 
 


Test patch:
diff --git a/stix2validator/test/observed_data_tests.py b/stix2validator/test/observed_data_tests.py
index 8505655..3a19210 100644
--- a/stix2validator/test/observed_data_tests.py
+++ b/stix2validator/test/observed_data_tests.py
@@ -763,6 +763,12 @@ class ObservedDataTestCases(ValidatorTest):
         }
         self.assertFalseWithOptions(json.dumps(observed_data))
 
+        del observed_data['objects']['2']['url']
+        self.assertTrueWithOptions(json.dumps(observed_data))
+
+        observed_data['objects']['2']['payload_bin'] = "failing test"
+        self.assertFalseWithOptions(json.dumps(observed_data))
+
     def test_file_invalid_is_encrypted(self):
         observed_data = copy.deepcopy(self.valid_observed_data)
         observed_data['objects']['2'] = {
@@ -775,6 +781,17 @@ class ObservedDataTestCases(ValidatorTest):
         }
         self.assertFalseWithOptions(json.dumps(observed_data))
 
+    def test_hash_length(self):
+        observed_data = copy.deepcopy(self.valid_observed_data)
+        hash_name = "abcdefghijklmnopqrstuvwxyz0123456789"
+        observed_data['objects']['0']['hashes'][hash_name] = "8D98A25E9D0662B1F4CA3BF22D6F53E9"
+        self.assertFalseWithOptions(json.dumps(observed_data))
+
+        observed_data = copy.deepcopy(self.valid_observed_data)
+        hash_name = "MD"
+        observed_data['objects']['0']['hashes'][hash_name] = "8D98A25E9D0662B1F4CA3BF22D6F53E9"
+        self.assertFalseWithOptions(json.dumps(observed_data))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/stix2validator/test/vulnerability_tests.py b/stix2validator/test/vulnerability_tests.py
index c392af7..403abe7 100644
--- a/stix2validator/test/vulnerability_tests.py
+++ b/stix2validator/test/vulnerability_tests.py
@@ -1,9 +1,8 @@
 import unittest
 import copy
 import json
-from . import SCHEMA_DIR
+from . import ValidatorTest
 from .. import validate_string
-from ..util import ValidationOptions
 
 VALID_VULNERABILITY = """
 {
@@ -22,9 +21,8 @@ VALID_VULNERABILITY = """
 """
 
 
-class VulnerabilityTestCases(unittest.TestCase):
+class VulnerabilityTestCases(ValidatorTest):
     valid_vulnerability = json.loads(VALID_VULNERABILITY)
-    options = ValidationOptions(schema_dir=SCHEMA_DIR)
 
     def test_wellformed_vulnerability(self):
         results = validate_string(VALID_VULNERABILITY, self.options)
@@ -70,6 +68,22 @@ class VulnerabilityTestCases(unittest.TestCase):
         results = validate_string(vulnerability, self.options)
         self.assertEqual(results.is_valid, False)
 
+    def test_url_no_hash(self):
+        vulnerability = copy.deepcopy(self.valid_vulnerability)
+        ext_refs = vulnerability['external_references']
+        ext_refs[0]['source_name'] = "veris"
+        ext_refs[0]['url'] = "https://github.com/vz-risk/VCDB/blob/master/" \
+                             "data/json/0001AA7F-C601-424A-B2B8-BE6C9F5164E7.json"
+        ext_refs[0]['external_id'] = "0001AA7F-C601-424A-B2B8-BE6C9F5164E7"
+        self.assertFalseWithOptions(json.dumps(vulnerability))
+
+        self.check_ignore(json.dumps(vulnerability), 'extref-hashes')
+
+        ext_refs[0]['hashes'] = {
+            "SHA-256": "6db12788c37247f2316052e142f42f4b259d6561751e5f401a1ae2a6df9c674b"
+        }
+        self.assertTrueWithOptions(json.dumps(vulnerability))
+
 
 if __name__ == "__main__":
     unittest.main()