Fix non-unique OWASP ZAP security warning keys. Fixes #1492.

ICTU · Sep 26, 2020 · e53f8e2 · e53f8e2
1 parent 9ab8037
commit e53f8e2
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 7 deletions.
diff --git a/components/collector/src/source_collectors/file_source_collectors/owasp_zap.py b/components/collector/src/source_collectors/file_source_collectors/owasp_zap.py
@@ -22,7 +22,7 @@ async def _parse_source_responses(self, responses: SourceResponses) -> SourceMea
         risks = cast(List[str], self._parameter("risks"))
         for alert in await self.__alerts(responses, risks):
             alert_key = ":".join(
-                [alert.findtext(id_tag, default="") for id_tag in ("pluginid", "cweid", "wascid", "sourceid")])
+                [alert.findtext(id_tag, default="") for id_tag in ("alert", "pluginid", "cweid", "wascid", "sourceid")])
             name = alert.findtext("name", default="")
             description = tag_re.sub("", alert.findtext("desc", default=""))
             risk = alert.findtext("riskdesc", default="")

diff --git a/components/collector/tests/source_collectors/file_source_collectors/test_owasp_zap.py b/components/collector/tests/source_collectors/file_source_collectors/test_owasp_zap.py
@@ -60,11 +60,11 @@ async def test_warnings(self):
         url2 = "http://www.hackazon.com/products_pictures/How_to_Marry_a_Millionaire.jpg"
         expected_entities = [
             dict(
-                key=md5_hash(f"10021:16:15:3:GET:{url1}"), name=self.warning_name, description=self.warning_description,
-                location=f"GET {url1}", uri=url1, risk=self.warning_risk),
+                key=md5_hash(f"X-Content-Type-Options Header Missing:10021:16:15:3:GET:{url1}"), name=self.warning_name,
+                description=self.warning_description, location=f"GET {url1}", uri=url1, risk=self.warning_risk),
             dict(
-                key=md5_hash(f"10021:16:15:3:GET:{url2}"), name=self.warning_name, description=self.warning_description,
-                location=f"GET {url2}", uri=url2, risk=self.warning_risk)]
+                key=md5_hash(f"X-Content-Type-Options Header Missing:10021:16:15:3:GET:{url2}"), name=self.warning_name,
+                description=self.warning_description, location=f"GET {url2}", uri=url2, risk=self.warning_risk)]
         self.assert_measurement(response, value="2", entities=expected_entities)
 
     async def test_variable_url_regexp(self):
@@ -75,8 +75,9 @@ async def test_variable_url_regexp(self):
         stable_url = "http://www.hackazon.com/products_pictures/variable-part-removed"
         expected_entities = [
             dict(
-                key=md5_hash(f"10021:16:15:3:GET:{stable_url}"), name=self.warning_name, uri=stable_url,
-                description=self.warning_description, location=f"GET {stable_url}", risk=self.warning_risk)]
+                key=md5_hash(f"X-Content-Type-Options Header Missing:10021:16:15:3:GET:{stable_url}"),
+                name=self.warning_name, uri=stable_url, description=self.warning_description,
+                location=f"GET {stable_url}", risk=self.warning_risk)]
         self.assert_measurement(response, value="1", entities=expected_entities)
 
     async def test_source_up_to_dateness(self):

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed
 
 - The measurement value and target of metrics with unit minutes and their scale set to percentage were formatted incorrectly (e.g. "0:50%" instead of "50%"). Fixes [#1480](https://github.com/ICTU/quality-time/issues/1480).
+- The security warnings in OWASP ZAP reports do not have unique keys. However, *Quality-time* needs security warnings to be uniquely identifiable to detect whether the list of warnings changes between measurements. Therefore, *Quality-time* generates keys for OWASP ZAP security warnings itself. Unfortunately, the key that *Quality-time* generated, was not guaranteed to be unique. NB: an unfortunate side effect of the fix is that any OWASP ZAP security warning marked as false positive, confirmed, fixed, or won't fix will return to the unconfirmed state. Fixes [#1492](https://github.com/ICTU/quality-time/issues/1492).
 
 ## [3.6.0] - [2020-09-19]