Add option to swallow parse errors during verification, fixes #17

docs/changelog.rst

@@ -8,16 +8,36 @@ v0.7.0 (unreleased)
   parsing of ASN.1 structures, adding the ability to parse structures independent of
   RFC version. Certain bugs bugs we've encountered in the past, have now been resolved
   as a result of this. On top of that, structures defined in the replacement,
-  ``asn1crypto`` are a lot more Pythonic, and parsing speed has been sliced in more
+  ``asn1crypto``, are a lot more Pythonic, and parsing speed has been sliced in more
   than half.
-* This does have a serious impact if you use certain functions to deeply inspect the
+
+  This does have a serious impact if you use certain functions to deeply inspect the
   original data (as all these structures have now changed) and on some parts of the API
   to better align with the new dependency. Most notably, all OIDs are now strings,
-  rather than integer tuples, and references to attributes and subclasses are now
-  strings as well (such as in attribute lists).
+  rather than integer tuples, and references to attributes or specific types are now
+  strings as well (such as in attribute lists). These strings can be in dotted form,
+  but most commonly are a representation as provided by ``asn1crypto`` or ourselves.
+
+* Add (default) option to swallow ``SignedPEParseError`` while parsing a PE file's
+  certificate table. This allows checking certificates until such a parse error occurs,
+  better aligning with how Windows handles these cases.
+
+  ``SignedPEFile.signed_datas`` will no longer raise an exception when anything goes
+  wrong, and will simply stop without yielding anything if no valid
+  ``AuthenticodeSignedData`` is found.
 
-* Add support for ``SignedData`` versions other than v1
+  ``SignedPEFile.verify`` will raise a ``AuthenticodeNotSignedError`` when there's no
+  valid ``AuthenticodeSignedData``, instead of a ``SignedPEParseError``.
+
+  The former behaviour can be restored with the ``ignore_parse_errors`` argument to
+  ``SignedPEFile.verify`` and ``SignedPEFile.iter_signed_datas``. The latter method
+  has been changed to keyword-arguments only.
+
+* Add support for ``AuthenticodeSignedData`` versions other than v1
 * Add support for ``SignerInfo`` versions other than v1
+* Fix bug that could cause out-of-bound reads during parsing of the PE file's
+  certificate table
+
 * Parse the ``SpcPeImageData`` as part of the SpcInfo. This adds the attributes
   ``image_flags`` and ``image_publisher``, although this information is never used.
 * Parse the ``SpcStatementType`` as part of the authenticated attributes of the

signify/authenticode/signed_pe.py

@@ -263,13 +263,27 @@ def signed_datas(self) -> Iterator[structures.AuthenticodeSignedData]:
         yield from self.iter_signed_datas()
 
     def iter_signed_datas(
-        self, include_nested: bool = True
+        self, *, include_nested: bool = True, ignore_parse_errors: bool = True
     ) -> Iterator[structures.AuthenticodeSignedData]:
         """Returns an iterator over :class:`AuthenticodeSignedData` objects relevant
         for this PE file.
 
         :param include_nested: Boolean, if True, will also iterate over all nested
             SignedData structures
+        :param ignore_parse_errors: Indicates how to handle
+            :exc:`SignedPEParseError` that may be raised while fetching
+            embedded :class:`structures.AuthenticodeSignedData` structures.
+
+            When :const:`True`,  which is the default and seems to be how Windows
+            handles this as well, this will fetch as many valid
+            :class:`structures.AuthenticodeSignedData` structures until an exception
+            occurs.
+
+            Note that this will also silence the :exc:`SignedPEParseError` that occurs
+            when there's no valid :class:`AuthenticodeSignedData` to fetch.
+
+            When :const:`False`, this will raise the :exc:`SignedPEParseError` as
+            soon as one occurs.
         :raises SignedPEParseError: For parse errors in the PEFile
         :raises signify.authenticode.AuthenticodeParseError: For parse errors in the
             SignedData
@@ -284,26 +298,30 @@ def recursive_nested(
                 for nested in signed_data.signer_info.nested_signed_datas:
                     yield from recursive_nested(nested)
 
-        found = False
-        for certificate in self._parse_cert_table():
-            if certificate["revision"] != 0x200:
-                raise SignedPEParseError(
-                    f"Unknown certificate revision {certificate['revision']!r}"
-                )
+        try:
+            found = False
+            for certificate in self._parse_cert_table():
+                if certificate["revision"] != 0x200:
+                    raise SignedPEParseError(
+                        f"Unknown certificate revision {certificate['revision']!r}"
+                    )
 
-            if certificate["type"] == 2:
-                yield from recursive_nested(
-                    structures.AuthenticodeSignedData.from_envelope(
-                        certificate["certificate"], pefile=self
+                if certificate["type"] == 2:
+                    yield from recursive_nested(
+                        structures.AuthenticodeSignedData.from_envelope(
+                            certificate["certificate"], pefile=self
+                        )
                     )
-                )
-                found = True
+                    found = True
 
-        if not found:
-            raise SignedPEParseError(
-                "A SignedData structure was not found in the PE file's Certificate"
-                " Table"
-            )
+            if not found:
+                raise SignedPEParseError(
+                    "A SignedData structure was not found in the PE file's Certificate"
+                    " Table"
+                )
+        except SignedPEParseError:
+            if not ignore_parse_errors:
+                raise
 
     def _calculate_expected_hashes(
         self,
@@ -345,6 +363,7 @@ def verify(
         *,
         multi_verify_mode: Literal["any", "first", "all", "best"] = "any",
         expected_hashes: dict[str, bytes] | None = None,
+        ignore_parse_errors: bool = True,
         **kwargs: Any,
     ) -> bool:
         """Verifies the SignedData structures. This is a little bit more efficient than
@@ -353,7 +372,7 @@ def verify(
         :param expected_hashes: When provided, should be a mapping of hash names to
             digests. This could speed up the verification process.
         :param multi_verify_mode: Indicates how to verify when there are multiple
-            :cls:`AuthenticodeSignedData` objects in this PE file. Can be:
+            :class:`structures.AuthenticodeSignedData` objects in this PE file. Can be:
 
             * 'any' (default) to indicate that any of the signatures must validate
               correctly.
@@ -366,11 +385,27 @@ def verify(
               any may verify
 
             This argument has no effect when only one signature is present.
+        :param ignore_parse_errors: Indicates how to handle :exc:`SignedPEParseError`
+            that may be raised during parsing of the PE file's certificate table.
+
+            When :const:`True`, which is the default and seems to be how Windows
+            handles this as well, this will verify based on all available
+            :class:`structures.AuthenticodeSignedData` before a parse error occurs.
+
+            :exc:`AuthenticodeNotSignedError` will be raised when no valid
+            :class:`structures.AuthenticodeSignedData` exists.
+
+            When :const:`False`, this will raise the :exc:`SignedPEParseError` as soon
+            as one occurs. This often occurs before :exc:`AuthenticodeNotSignedError`
+            is potentially raised.
         :raises AuthenticodeVerificationError: when the verification failed
+        :raises SignedPEParseError: for parse errors in the PEFile
         """
 
         # we need to iterate it twice, so we need to prefetch all signed_datas
-        signed_datas = list(self.signed_datas)
+        signed_datas = list(
+            self.iter_signed_datas(ignore_parse_errors=ignore_parse_errors)
+        )
 
         # if there are no signed_datas, the binary is not signed
         if not signed_datas:

tests/test_authenticode.py

@@ -36,6 +36,7 @@
     AuthenticodeVerificationError,
     SignedPEParseError,
     VerificationError,
+    AuthenticodeNotSignedError,
 )
 from signify.fingerprinter import AuthenticodeFingerprinter
 from signify.x509.context import (
@@ -101,8 +102,12 @@ def test_modified_pciide_fails(self):
     def test_simple(self):
         with open(str(root_dir / "test_data" / "simple"), "rb") as f:
             pefile = SignedPEFile(f)
-            self.assertRaises(SignedPEParseError, list, pefile.signed_datas)
-            self.assertRaises(SignedPEParseError, pefile.verify)
+            self.assertRaises(AuthenticodeNotSignedError, pefile.verify)
+            self.assertRaises(
+                SignedPEParseError,
+                list,
+                pefile.iter_signed_datas(ignore_parse_errors=False),
+            )
 
     def test_2A6E(self):
         with open(str(root_dir / "test_data" / "___2A6E.tmp"), "rb") as f: