fix(iast): invalid line number on insecure cookie report (#6887)

Insecure Cookies report should not send line number and empty file

## 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.

Author: avara1986

ddtrace/appsec/iast/_taint_tracking/__init__.py

@@ -70,6 +70,7 @@
     "active_map_addreses_size",
     "create_context",
     "str_to_origin",
+    "origin_to_str",
     "common_replace",
     "as_formatted_evidence",
     "parse_params",

ddtrace/appsec/iast/_utils.py

@@ -1,8 +1,11 @@
+import json
 import re
 import string
 import sys
 from typing import TYPE_CHECKING
 
+import attr
+
 from ddtrace.internal.logger import get_logger
 from ddtrace.settings import _config as config
 
@@ -72,3 +75,17 @@ def _scrub_get_tokens_positions(text, tokens):
 
     token_positions.sort()
     return token_positions
+
+
+def _iast_report_to_str(data):
+    from ._taint_tracking import OriginType
+    from ._taint_tracking import origin_to_str
+
+    class OriginTypeEncoder(json.JSONEncoder):
+        def default(self, obj):
+            if isinstance(obj, OriginType):
+                # if the obj is uuid, we simply return the value of uuid
+                return origin_to_str(obj)
+            return json.JSONEncoder.default(self, obj)
+
+    return json.dumps(attr.asdict(data, filter=lambda attr, x: x is not None), cls=OriginTypeEncoder)

ddtrace/appsec/iast/processor.py

@@ -1,4 +1,3 @@
-import json
 from typing import TYPE_CHECKING
 
 import attr
@@ -7,6 +6,7 @@
 from ddtrace.appsec._constants import IAST
 from ddtrace.appsec.iast import oce
 from ddtrace.appsec.iast._metrics import _set_metric_iast_request_tainted
+from ddtrace.appsec.iast._utils import _iast_report_to_str
 from ddtrace.appsec.iast._utils import _is_iast_enabled
 from ddtrace.appsec.trace_utils import _asm_manual_keep
 from ddtrace.constants import ORIGIN_KEY
@@ -56,19 +56,9 @@ def on_span_finish(self, span):
         data = core.get_item(IAST.CONTEXT_KEY, span=span)
 
         if data:
-            from ddtrace.appsec.iast._taint_tracking import OriginType  # noqa: F401
-            from ddtrace.appsec.iast._taint_tracking._native.taint_tracking import origin_to_str  # noqa: F401
-
-            class OriginTypeEncoder(json.JSONEncoder):
-                def default(self, obj):
-                    if isinstance(obj, OriginType):
-                        # if the obj is uuid, we simply return the value of uuid
-                        return origin_to_str(obj)
-                    return json.JSONEncoder.default(self, obj)
-
             span.set_tag_str(
                 IAST.JSON,
-                json.dumps(attr.asdict(data, filter=lambda attr, x: x is not None), cls=OriginTypeEncoder),
+                _iast_report_to_str(data),
             )
             _asm_manual_keep(span)
 

ddtrace/appsec/iast/reporter.py

@@ -55,9 +55,9 @@ def __eq__(self, other):
 
 @attr.s(eq=True, hash=True)
 class Location(object):
-    path = attr.ib(type=str)  # type: str
-    line = attr.ib(type=int)  # type: int
     spanId = attr.ib(type=int, eq=False, hash=False, repr=False)  # type: int
+    path = attr.ib(type=str, default=None)  # type: Optional[str]
+    line = attr.ib(type=int, default=None)  # type: Optional[int]
 
 
 @attr.s(eq=True, hash=True)

ddtrace/appsec/iast/taint_sinks/_base.py

@@ -104,8 +104,8 @@ def report(cls, evidence_value="", sources=None):
                 )
                 return None
 
-            file_name = ""
-            line_number = 0
+            file_name = None
+            line_number = None
 
             skip_location = getattr(cls, "skip_location", False)
             if not skip_location:
@@ -119,6 +119,9 @@ def report(cls, evidence_value="", sources=None):
                 if file_name.startswith(CWD):
                     file_name = os.path.relpath(file_name, start=CWD)
 
+                if not cls.is_not_reported(file_name, line_number):
+                    return
+
             if _is_evidence_value_parts(evidence_value):
                 evidence = Evidence(valueParts=evidence_value)
             # Evidence is a string in weak cipher, weak hash and weak randomness
@@ -128,10 +131,6 @@ def report(cls, evidence_value="", sources=None):
                 log.debug("Unexpected evidence_value type: %s", type(evidence_value))
                 evidence = Evidence(value="")
 
-            if not cls.is_not_reported(file_name, line_number):
-                # not not reported = reported
-                return None
-
             _set_metric_iast_executed_sink(cls.vulnerability_type)
 
             report = core.get_item(IAST.CONTEXT_KEY, span=span)

tests/appsec/iast/taint_sinks/test_insecure_cookie.py

@@ -1,4 +1,8 @@
+import pytest
+
 from ddtrace.appsec._constants import IAST
+from ddtrace.appsec.iast._utils import _iast_report_to_str
+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
@@ -14,12 +18,19 @@ def test_insecure_cookies(iast_span_defaults):
     assert list(span_report.vulnerabilities)[0].evidence.value == "foo=bar"
 
 
+@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
+    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
+    assert '"path"' not in str_report
 
 
 def test_nosamesite_cookies_missing(iast_span_defaults):