diff --git a/ddtrace/appsec/_iast/taint_sinks/header_injection.py b/ddtrace/appsec/_iast/taint_sinks/header_injection.py index 730e9f05490..da3c9c8e330 100644 --- a/ddtrace/appsec/_iast/taint_sinks/header_injection.py +++ b/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) diff --git a/lib-injection/sources/min_compatible_versions.csv b/lib-injection/sources/min_compatible_versions.csv index b49ced9c42e..4537863f24c 100644 --- a/lib-injection/sources/min_compatible_versions.csv +++ b/lib-injection/sources/min_compatible_versions.csv @@ -24,6 +24,7 @@ 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 @@ -31,6 +32,7 @@ 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 diff --git a/min_compatible_versions.csv b/min_compatible_versions.csv index e4a9d8e22bf..bdc06d83f50 100644 --- a/min_compatible_versions.csv +++ b/min_compatible_versions.csv @@ -24,6 +24,7 @@ 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 @@ -31,6 +32,7 @@ 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 diff --git a/releasenotes/notes/fix-lib-injection-python-json-logger-5248a251acb1adf4.yaml b/releasenotes/notes/fix-lib-injection-python-json-logger-5248a251acb1adf4.yaml new file mode 100644 index 00000000000..916bdd88afe --- /dev/null +++ b/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. diff --git a/releasenotes/notes/iast-fastapi-header-injection-ce4805c91e87ebe2.yaml b/releasenotes/notes/iast-fastapi-header-injection-ce4805c91e87ebe2.yaml new file mode 100644 index 00000000000..6b478151e88 --- /dev/null +++ b/releasenotes/notes/iast-fastapi-header-injection-ce4805c91e87ebe2.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Code Security: add support for Header Injection vulnerability sink point. diff --git a/scripts/min_compatible_versions.py b/scripts/min_compatible_versions.py index ae35049f02b..88d955e8f5f 100644 --- a/scripts/min_compatible_versions.py +++ b/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]: diff --git a/tests/contrib/fastapi/test_fastapi_appsec_iast.py b/tests/contrib/fastapi/test_fastapi_appsec_iast.py index 980a1297a69..dd3050771c6 100644 --- a/tests/contrib/fastapi/test_fastapi_appsec_iast.py +++ b/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 diff --git a/tests/debugging/exception/test_replay.py b/tests/debugging/exception/test_replay.py index 54baeb8b826..9aae75dae47 100644 --- a/tests/debugging/exception/test_replay.py +++ b/tests/debugging/exception/test_replay.py @@ -123,7 +123,7 @@ def c(foo=42): for n, span in enumerate(self.spans): assert span.get_tag(replay.DEBUG_INFO_TAG) == "true" - exc_id = span.get_tag("_dd.debug.error.exception_id") + exc_id = span.get_tag(replay.EXCEPTION_ID_TAG) info = {k: v for k, v in enumerate(["c", "b", "a"][n:], start=1)} @@ -147,8 +147,8 @@ def c(foo=42): assert all(str(s.exc_id) == exc_id for s in snapshots.values()) # assert all spans use the same exc_id - exc_ids = set(span.get_tag("_dd.debug.error.exception_id") for span in self.spans) - assert len(exc_ids) == 1 + exc_ids = set(span.get_tag(replay.EXCEPTION_ID_TAG) for span in self.spans) + assert None not in exc_ids and len(exc_ids) == 1 def test_debugger_exception_chaining(self): def a(v, d=None): @@ -190,7 +190,7 @@ def c(foo=42): for n, span in enumerate(self.spans): assert span.get_tag(replay.DEBUG_INFO_TAG) == "true" - exc_id = span.get_tag("_dd.debug.error.exception_id") + exc_id = span.get_tag(replay.EXCEPTION_ID_TAG) info = {k: v for k, v in enumerate(stacks[n], start=1)} @@ -215,8 +215,8 @@ def c(foo=42): assert any(str(s.exc_id) == exc_id for s in snapshots.values()) # assert number of unique exc_ids based on python version - exc_ids = set(span.get_tag("_dd.debug.error.exception_id") for span in self.spans) - assert len(exc_ids) == number_of_exc_ids + exc_ids = set(span.get_tag(replay.EXCEPTION_ID_TAG) for span in self.spans) + assert None not in exc_ids and len(exc_ids) == number_of_exc_ids # invoke again (should be in less than 1 sec) with with_rate_limiter(rate_limiter):