fix(iast): report cookies vuln [backport 2.0] (#7499)

Backport 58a5968 from #7497 to 2.0.

Fix Cookie vulnerability errors: When IAST finds ONE of the Cookie
vulnerabilities, it stops and returns the report. IAST should check all
possible Cookie vulnerabilities and report if any one, two, or all three
types of Cookie vulnerabilities are found.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

Co-authored-by: Alberto Vara <[email protected]>

Author: github-actions[bot]

ddtrace/appsec/_iast/taint_sinks/insecure_cookie.py

@@ -49,12 +49,10 @@ def asm_check_cookies(cookies):  # type: (Optional[Dict[str, str]]) -> None
         if ";secure" not in lvalue:
             _set_metric_iast_executed_sink(InsecureCookie.vulnerability_type)
             InsecureCookie.report(evidence_value=evidence)
-            return
 
         if ";httponly" not in lvalue:
             _set_metric_iast_executed_sink(NoHttpOnlyCookie.vulnerability_type)
             NoHttpOnlyCookie.report(evidence_value=evidence)
-            return
 
         if ";samesite=" in lvalue:
             ss_tokens = lvalue.split(";samesite=")

tests/appsec/iast/taint_sinks/test_insecure_cookie.py

@@ -14,19 +14,39 @@ def test_insecure_cookies(iast_span_defaults):
     cookies = {"foo": "bar"}
     asm_check_cookies(cookies)
     span_report = core.get_item(IAST.CONTEXT_KEY, span=iast_span_defaults)
-    assert list(span_report.vulnerabilities)[0].type == VULN_INSECURE_COOKIE
-    assert list(span_report.vulnerabilities)[0].evidence.value == "foo=bar"
+    vulnerabilities = list(span_report.vulnerabilities)
+    vulnerabilities_types = [vuln.type for vuln in vulnerabilities]
+    assert len(vulnerabilities) == 3
+    assert VULN_NO_HTTPONLY_COOKIE in vulnerabilities_types
+    assert VULN_INSECURE_COOKIE in vulnerabilities_types
+    assert VULN_NO_SAMESITE_COOKIE in vulnerabilities_types
+
+    assert vulnerabilities[0].evidence.value == "foo=bar"
+    assert vulnerabilities[1].evidence.value == "foo=bar"
+    assert vulnerabilities[2].evidence.value == "foo=bar"
+
+    assert vulnerabilities[0].location.line is None
+    assert vulnerabilities[0].location.path is None
 
 
 @pytest.mark.skipif(not python_supported_by_iast(), reason="Python version not supported by IAST")
 def test_nohttponly_cookies(iast_span_defaults):
     cookies = {"foo": "bar;secure"}
     asm_check_cookies(cookies)
     span_report = core.get_item(IAST.CONTEXT_KEY, span=iast_span_defaults)
-    assert list(span_report.vulnerabilities)[0].type == VULN_NO_HTTPONLY_COOKIE
-    assert list(span_report.vulnerabilities)[0].evidence.value == "foo=bar;secure"
-    assert list(span_report.vulnerabilities)[0].location.line is None
-    assert list(span_report.vulnerabilities)[0].location.path is None
+
+    vulnerabilities = list(span_report.vulnerabilities)
+    vulnerabilities_types = [vuln.type for vuln in vulnerabilities]
+    assert len(vulnerabilities) == 2
+    assert VULN_NO_HTTPONLY_COOKIE in vulnerabilities_types
+    assert VULN_NO_SAMESITE_COOKIE in vulnerabilities_types
+
+    assert vulnerabilities[0].evidence.value == "foo=bar;secure"
+    assert vulnerabilities[1].evidence.value == "foo=bar;secure"
+
+    assert vulnerabilities[0].location.line is None
+    assert vulnerabilities[0].location.path is None
+
     str_report = _iast_report_to_str(span_report)
     # Double check to verify we're not sending an empty key
     assert '"line"' not in str_report
@@ -36,26 +56,40 @@ def test_nohttponly_cookies(iast_span_defaults):
 def test_nosamesite_cookies_missing(iast_span_defaults):
     cookies = {"foo": "bar;secure;httponly"}
     asm_check_cookies(cookies)
+
     span_report = core.get_item(IAST.CONTEXT_KEY, span=iast_span_defaults)
-    assert list(span_report.vulnerabilities)[0].type == VULN_NO_SAMESITE_COOKIE
-    assert list(span_report.vulnerabilities)[0].evidence.value == "foo=bar;secure;httponly"
+
+    vulnerabilities = list(span_report.vulnerabilities)
+
+    assert len(vulnerabilities) == 1
+    assert vulnerabilities[0].type == VULN_NO_SAMESITE_COOKIE
+    assert vulnerabilities[0].evidence.value == "foo=bar;secure;httponly"
 
 
 def test_nosamesite_cookies_none(iast_span_defaults):
     cookies = {"foo": "bar;secure;httponly;samesite=none"}
     asm_check_cookies(cookies)
     span_report = core.get_item(IAST.CONTEXT_KEY, span=iast_span_defaults)
-    assert list(span_report.vulnerabilities)[0].type == VULN_NO_SAMESITE_COOKIE
 
-    assert list(span_report.vulnerabilities)[0].evidence.value == "foo=bar;secure;httponly;samesite=none"
+    vulnerabilities = list(span_report.vulnerabilities)
+
+    assert len(vulnerabilities) == 1
+
+    assert vulnerabilities[0].type == VULN_NO_SAMESITE_COOKIE
+    assert vulnerabilities[0].evidence.value == "foo=bar;secure;httponly;samesite=none"
 
 
 def test_nosamesite_cookies_other(iast_span_defaults):
     cookies = {"foo": "bar;secure;httponly;samesite=none"}
     asm_check_cookies(cookies)
     span_report = core.get_item(IAST.CONTEXT_KEY, span=iast_span_defaults)
-    assert list(span_report.vulnerabilities)[0].type == VULN_NO_SAMESITE_COOKIE
-    assert list(span_report.vulnerabilities)[0].evidence.value == "foo=bar;secure;httponly;samesite=none"
+
+    vulnerabilities = list(span_report.vulnerabilities)
+
+    assert len(vulnerabilities) == 1
+
+    assert vulnerabilities[0].type == VULN_NO_SAMESITE_COOKIE
+    assert vulnerabilities[0].evidence.value == "foo=bar;secure;httponly;samesite=none"
 
 
 def test_nosamesite_cookies_lax_no_error(iast_span_defaults):

tests/contrib/flask/test_flask_appsec_iast.py

@@ -6,6 +6,9 @@
 from ddtrace.appsec._constants import IAST
 from ddtrace.appsec._iast import oce
 from ddtrace.appsec._iast._utils import _is_python_version_supported as python_supported_by_iast
+from ddtrace.appsec._iast.constants import VULN_INSECURE_COOKIE
+from ddtrace.appsec._iast.constants import VULN_NO_HTTPONLY_COOKIE
+from ddtrace.appsec._iast.constants import VULN_NO_SAMESITE_COOKIE
 from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
 from ddtrace.contrib.sqlite3.patch import patch
 from tests.appsec.iast.iast_utils import get_line_and_hash
@@ -430,7 +433,12 @@ def test_sqli():
                     assert vulnerability["location"]["path"] == TEST_FILE_PATH
                     assert vulnerability["hash"] == hash_value
 
-            assert {VULN_SQL_INJECTION, "INSECURE_COOKIE"} == vulnerabilities
+            assert {
+                VULN_SQL_INJECTION,
+                VULN_INSECURE_COOKIE,
+                VULN_NO_HTTPONLY_COOKIE,
+                VULN_NO_SAMESITE_COOKIE,
+            } == vulnerabilities
 
     @pytest.mark.skipif(not python_supported_by_iast(), reason="Python version not supported by IAST")
     def test_flask_full_sqli_iast_http_request_parameter(self):