Merge branch 'main' into brettlangdon/es.flaky

ddtrace/appsec/_iast/taint_sinks/header_injection.py

@@ -1,3 +1,4 @@
+import typing
 from typing import Text
 
 from wrapt.importer import when_imported
@@ -51,6 +52,8 @@ def patch():
         return
     if not set_and_check_module_is_patched("django", default_attr="_datadog_header_injection_patch"):
         return
+    if not set_and_check_module_is_patched("fastapi", default_attr="_datadog_header_injection_patch"):
+        return
 
     @when_imported("wsgiref.headers")
     def _(m):
@@ -68,6 +71,16 @@ def _(m):
         try_wrap_function_wrapper(m, "HttpResponseBase.__setitem__", _iast_h)
         try_wrap_function_wrapper(m, "ResponseHeaders.__setitem__", _iast_h)
 
+    # For headers["foo"] = "bar"
+    @when_imported("starlette.datastructures")
+    def _(m):
+        try_wrap_function_wrapper(m, "MutableHeaders.__setitem__", _iast_h)
+
+    # For Response("ok", header=...)
+    @when_imported("starlette.responses")
+    def _(m):
+        try_wrap_function_wrapper(m, "Response.init_headers", _iast_h)
+
     _set_metric_iast_instrumented_sink(VULN_HEADER_INJECTION)
 
 
@@ -78,15 +91,16 @@ def unpatch():
     try_unwrap("werkzeug.datastructures", "Headers.add")
     try_unwrap("django.http.response", "HttpResponseBase.__setitem__")
     try_unwrap("django.http.response", "ResponseHeaders.__setitem__")
+    try_unwrap("starlette.datastructures", "MutableHeaders.__setitem__")
+    try_unwrap("starlette.responses", "Response.init_headers")
 
     set_module_unpatched("flask", default_attr="_datadog_header_injection_patch")
     set_module_unpatched("django", default_attr="_datadog_header_injection_patch")
-
-    pass
+    set_module_unpatched("fastapi", default_attr="_datadog_header_injection_patch")
 
 
 def _iast_h(wrapped, instance, args, kwargs):
-    if asm_config._iast_enabled:
+    if asm_config._iast_enabled and args:
         _iast_report_header_injection(args)
     if hasattr(wrapped, "__func__"):
         return wrapped.__func__(instance, *args, **kwargs)
@@ -98,10 +112,16 @@ class HeaderInjection(VulnerabilityBase):
     vulnerability_type = VULN_HEADER_INJECTION
 
 
-def _iast_report_header_injection(headers_args) -> None:
+def _process_header(headers_args):
     from ddtrace.appsec._iast._taint_tracking.aspects import add_aspect
 
+    if len(headers_args) != 2:
+        return
+
     header_name, header_value = headers_args
+    if header_name is None:
+        return
+
     for header_to_exclude in HEADER_INJECTION_EXCLUSIONS:
         header_name_lower = header_name.lower()
         if header_name_lower == header_to_exclude or header_name_lower.startswith(header_to_exclude):
@@ -114,3 +134,15 @@ def _iast_report_header_injection(headers_args) -> None:
         if is_pyobject_tainted(header_name) or is_pyobject_tainted(header_value):
             header_evidence = add_aspect(add_aspect(header_name, HEADER_NAME_VALUE_SEPARATOR), header_value)
             HeaderInjection.report(evidence_value=header_evidence)
+
+
+def _iast_report_header_injection(headers_or_args) -> None:
+    if headers_or_args and isinstance(headers_or_args[0], typing.Mapping):
+        # ({header_name: header_value}, {header_name: header_value}, ...), used by FastAPI Response constructor
+        # when used with Response(..., headers={...})
+        for headers_dict in headers_or_args:
+            for header_name, header_value in headers_dict.items():
+                _process_header((header_name, header_value))
+    else:
+        # (header_name, header_value), used in other cases
+        _process_header(headers_or_args)

ddtrace/internal/telemetry/writer.py

@@ -118,11 +118,17 @@ def send_event(self, request: Dict) -> Optional[httplib.HTTPResponse]:
                 conn.request("POST", self._endpoint, rb_json, headers)
                 resp = get_connection_response(conn)
             if resp.status < 300:
-                log.debug("sent %d in %.5fs to %s. response: %s", len(rb_json), sw.elapsed(), self.url, resp.status)
+                log.debug(
+                    "Instrumentation Telemetry sent %d in %.5fs to %s. response: %s",
+                    len(rb_json),
+                    sw.elapsed(),
+                    self.url,
+                    resp.status,
+                )
             else:
-                log.debug("failed to send telemetry to %s. response: %s", self.url, resp.status)
-        except Exception:
-            log.debug("failed to send telemetry to %s.", self.url, exc_info=True)
+                log.debug("Failed to send Instrumentation Telemetry to %s. response: %s", self.url, resp.status)
+        except Exception as e:
+            log.debug("Failed to send Instrumentation Telemetry to %s. Error: %s", self.url, str(e))
         finally:
             if conn is not None:
                 conn.close()

lib-injection/sources/min_compatible_versions.csv

@@ -24,13 +24,15 @@ asyncpg,~=0.23
 asynctest,==0.13.0
 austin-python,~=1.0
 avro,0
+azure.functions,0
 blinker,0
 boto3,==1.34.49
 bottle,>=0.12
 bytecode,0
 cassandra-driver,~=3.24.0
 cattrs,<23.1.1
 celery,~=5.1.0
+celery[redis],0
 cfn-lint,~=0.53.1
 channels,~=3.0
 cherrypy,>=17
@@ -68,12 +70,13 @@ flask,~=0.12.0
 flask-caching,~=1.10.0
 flask-openapi3,0
 gevent,~=20.12.0
+google-ai-generativelanguage,0
 google-generativeai,0
 googleapis-common-protos,0
 graphene,~=3.0.0
 graphql-core,~=3.2.0
 graphql-relay,0
-greenlet,~=1.0
+greenlet,~=1.0.0
 grpcio,~=1.34.0
 gunicorn,==20.0.4
 gunicorn[gevent],0
@@ -97,9 +100,10 @@ langchain-pinecone,==0.1.0
 langchain_experimental,==0.0.47
 logbook,~=1.0.0
 loguru,~=0.4.0
+lxml,0
 lz4,0
 mako,~=1.1.0
-mariadb,~=1.0
+mariadb,~=1.0.0
 markupsafe,<2.0
 mock,0
 molten,>=1.0
@@ -149,7 +153,6 @@ pytest-memray,~=1.7.0
 pytest-mock,==2.0.0
 pytest-sanic,~=1.6.2
 python-consul,>=1.1
-python-json-logger,==2.0.7
 python-memcached,0
 python-multipart,0
 ragas,==0.1.21
@@ -180,6 +183,7 @@ typing_extensions,0
 urllib3,~=1.0
 uwsgi,0
 vcrpy,==4.2.1
+vertexai,0
 vertica-python,>=0.6.0
 virtualenv-clone,0
 websockets,<11.0

min_compatible_versions.csv

@@ -24,13 +24,15 @@ asyncpg,~=0.23
 asynctest,==0.13.0
 austin-python,~=1.0
 avro,0
+azure.functions,0
 blinker,0
 boto3,==1.34.49
 bottle,>=0.12
 bytecode,0
 cassandra-driver,~=3.24.0
 cattrs,<23.1.1
 celery,~=5.1.0
+celery[redis],0
 cfn-lint,~=0.53.1
 channels,~=3.0
 cherrypy,>=17
@@ -68,12 +70,13 @@ flask,~=0.12.0
 flask-caching,~=1.10.0
 flask-openapi3,0
 gevent,~=20.12.0
+google-ai-generativelanguage,0
 google-generativeai,0
 googleapis-common-protos,0
 graphene,~=3.0.0
 graphql-core,~=3.2.0
 graphql-relay,0
-greenlet,~=1.0
+greenlet,~=1.0.0
 grpcio,~=1.34.0
 gunicorn,==20.0.4
 gunicorn[gevent],0
@@ -97,9 +100,10 @@ langchain-pinecone,==0.1.0
 langchain_experimental,==0.0.47
 logbook,~=1.0.0
 loguru,~=0.4.0
+lxml,0
 lz4,0
 mako,~=1.1.0
-mariadb,~=1.0
+mariadb,~=1.0.0
 markupsafe,<2.0
 mock,0
 molten,>=1.0
@@ -149,7 +153,6 @@ pytest-memray,~=1.7.0
 pytest-mock,==2.0.0
 pytest-sanic,~=1.6.2
 python-consul,>=1.1
-python-json-logger,==2.0.7
 python-memcached,0
 python-multipart,0
 ragas,==0.1.21
@@ -180,6 +183,7 @@ typing_extensions,0
 urllib3,~=1.0
 uwsgi,0
 vcrpy,==4.2.1
+vertexai,0
 vertica-python,>=0.6.0
 virtualenv-clone,0
 websockets,<11.0

releasenotes/notes/fix-lib-injection-python-json-logger-5248a251acb1adf4.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    lib-injection: remove python-json-logger from library compatibility check.

releasenotes/notes/iast-fastapi-header-injection-ce4805c91e87ebe2.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    Code Security: add support for Header Injection vulnerability sink point.

scripts/min_compatible_versions.py

@@ -14,7 +14,16 @@
 
 OUT_FILENAME = "min_compatible_versions.csv"
 OUT_DIRECTORIES = (".", "lib-injection/sources")
-IGNORED_PACKAGES = {"setuptools", "attrs", "pytest-randomly", "pillow", "botocore", "pytest-asyncio", "click"}
+IGNORED_PACKAGES = {
+    "attrs",
+    "botocore",
+    "click",
+    "pillow",
+    "pytest-asyncio",
+    "pytest-randomly",
+    "python-json-logger",
+    "setuptools",
+}
 
 
 def _format_version_specifiers(spec: Set[str]) -> Set[str]:

tests/contrib/fastapi/test_fastapi_appsec_iast.py

@@ -13,10 +13,13 @@
 from fastapi import __version__ as _fastapi_version
 from fastapi.responses import JSONResponse
 import pytest
+from starlette.responses import PlainTextResponse
 
 from ddtrace.appsec._constants import IAST
 from ddtrace.appsec._iast import oce
 from ddtrace.appsec._iast._handlers import _on_iast_fastapi_patch
+from ddtrace.appsec._iast._patch_modules import patch_iast
+from ddtrace.appsec._iast.constants import VULN_HEADER_INJECTION
 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
@@ -764,3 +767,77 @@ def insecure_cookie(request: Request):
         assert "line" not in vulnerability["location"].keys()
         assert vulnerability["location"]["spanId"]
         assert vulnerability["hash"]
+
+
+def test_fastapi_header_injection(fastapi_application, client, tracer, test_spans):
+    @fastapi_application.get("/header_injection/")
+    async def header_injection(request: Request):
+        from ddtrace.appsec._iast._taint_tracking._taint_objects import is_pyobject_tainted
+
+        tainted_string = request.headers.get("test")
+        assert is_pyobject_tainted(tainted_string)
+        result_response = JSONResponse(content={"message": "OK"})
+        # label test_fastapi_header_injection
+        result_response.headers["Header-Injection"] = tainted_string
+        result_response.headers["Vary"] = tainted_string
+        result_response.headers["Foo"] = "bar"
+
+        return result_response
+
+    with override_global_config(dict(_iast_enabled=True, _deduplication_enabled=False, _iast_request_sampling=100.0)):
+        _aux_appsec_prepare_tracer(tracer)
+        patch_iast({"header_injection": True})
+        resp = client.get(
+            "/header_injection/",
+            headers={"test": "test_injection_header"},
+        )
+        assert resp.status_code == 200
+
+        span = test_spans.pop_traces()[0][0]
+        assert span.get_metric(IAST.ENABLED) == 1.0
+
+        iast_tag = span.get_tag(IAST.JSON)
+        assert iast_tag is not None
+        loaded = json.loads(iast_tag)
+        line, hash_value = get_line_and_hash(
+            "test_fastapi_header_injection", VULN_HEADER_INJECTION, filename=TEST_FILE_PATH
+        )
+        assert len(loaded["vulnerabilities"]) == 1
+        vulnerability = loaded["vulnerabilities"][0]
+        assert vulnerability["type"] == VULN_HEADER_INJECTION
+        assert vulnerability["hash"] == hash_value
+        assert vulnerability["location"]["line"] == line
+        assert vulnerability["location"]["path"] == TEST_FILE_PATH
+        assert vulnerability["location"]["spanId"]
+
+
+def test_fastapi_header_injection_inline_response(fastapi_application, client, tracer, test_spans):
+    @fastapi_application.get("/header_injection_inline_response/", response_class=PlainTextResponse)
+    async def header_injection_inline_response(request: Request):
+        from ddtrace.appsec._iast._taint_tracking._taint_objects import is_pyobject_tainted
+
+        tainted_string = request.headers.get("test")
+        assert is_pyobject_tainted(tainted_string)
+        return PlainTextResponse(
+            content="OK",
+            headers={"Header-Injection": tainted_string, "Vary": tainted_string, "Foo": "bar"},
+        )
+
+    with override_global_config(dict(_iast_enabled=True, _deduplication_enabled=False, _iast_request_sampling=100.0)):
+        _aux_appsec_prepare_tracer(tracer)
+        patch_iast({"header_injection": True})
+        resp = client.get(
+            "/header_injection_inline_response/",
+            headers={"test": "test_injection_header"},
+        )
+        assert resp.status_code == 200
+
+        span = test_spans.pop_traces()[0][0]
+        assert span.get_metric(IAST.ENABLED) == 1.0
+
+        iast_tag = span.get_tag(IAST.JSON)
+        assert iast_tag is not None
+        loaded = json.loads(iast_tag)
+        assert len(loaded["vulnerabilities"]) == 1
+        vulnerability = loaded["vulnerabilities"][0]
+        assert vulnerability["type"] == VULN_HEADER_INJECTION

tests/contrib/urllib3/test_urllib3.py

@@ -12,6 +12,7 @@
 from ddtrace.ext import http
 from ddtrace.internal.schema import DEFAULT_SPAN_SERVICE_NAME
 from ddtrace.pin import Pin
+from ddtrace.settings.asm import config as asm_config
 from tests.contrib.config import HTTPBIN_CONFIG
 from tests.opentracer.utils import init_tracer
 from tests.utils import TracerTestCase
@@ -527,12 +528,16 @@ def test_distributed_tracing_disabled(self):
                     timeout=mock.ANY,
                 )
 
+    @pytest.mark.skip(reason="urlib3 does not set the ASM Manual keep tag so x-datadog headers are not propagated")
     def test_distributed_tracing_apm_opt_out_true(self):
         """Tests distributed tracing headers are passed by default"""
         # Check that distributed tracing headers are passed down; raise an error rather than make the
         # request since we don't care about the response at all
         config.urllib3["distributed_tracing"] = True
         self.tracer.enabled = False
+        # Ensure the ASM SpanProcessor is set
+        self.tracer.configure(appsec_standalone_enabled=True, appsec_enabled=True)
+        assert asm_config._apm_opt_out
         with mock.patch(
             "urllib3.connectionpool.HTTPConnectionPool._make_request", side_effect=ValueError
         ) as m_make_request:
@@ -580,6 +585,9 @@ def test_distributed_tracing_apm_opt_out_false(self):
         """Test with distributed tracing disabled does not propagate the headers"""
         config.urllib3["distributed_tracing"] = True
         self.tracer.enabled = False
+        # Ensure the ASM SpanProcessor is set.
+        self.tracer.configure(appsec_standalone_enabled=False, appsec_enabled=True)
+        assert not asm_config._apm_opt_out
         with mock.patch(
             "urllib3.connectionpool.HTTPConnectionPool._make_request", side_effect=ValueError
         ) as m_make_request: