diff --git a/ddtrace/appsec/_common_module_patches.py b/ddtrace/appsec/_common_module_patches.py index e077e62b54e..effd8473011 100644 --- a/ddtrace/appsec/_common_module_patches.py +++ b/ddtrace/appsec/_common_module_patches.py @@ -10,6 +10,8 @@ from ddtrace.appsec._constants import WAF_ACTIONS from ddtrace.appsec._constants import WAF_CONTEXT_NAMES +from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink +from ddtrace.appsec._iast.constants import VULN_PATH_TRAVERSAL from ddtrace.internal import core from ddtrace.internal._exceptions import BlockingException from ddtrace.internal.logger import get_logger @@ -26,6 +28,8 @@ def patch_common_modules(): try_wrap_function_wrapper("builtins", "open", wrapped_open_CFDDB7ABBA9081B6) try_wrap_function_wrapper("urllib.request", "OpenerDirector.open", wrapped_open_ED4CF71136E15EBF) + if asm_config._iast_enabled: + _set_metric_iast_instrumented_sink(VULN_PATH_TRAVERSAL) def unpatch_common_modules(): @@ -38,8 +42,9 @@ def wrapped_open_CFDDB7ABBA9081B6(original_open_callable, instance, args, kwargs wrapper for open file function """ if asm_config._iast_enabled: - # LFI sink to be added - pass + from ddtrace.appsec._iast.taint_sinks.path_traversal import check_and_report_path_traversal + + check_and_report_path_traversal(*args, **kwargs) if asm_config._asm_enabled and asm_config._ep_enabled: try: diff --git a/ddtrace/appsec/_iast/_ast/visitor.py b/ddtrace/appsec/_iast/_ast/visitor.py index 3664eb0f16d..534e721abdf 100644 --- a/ddtrace/appsec/_iast/_ast/visitor.py +++ b/ddtrace/appsec/_iast/_ast/visitor.py @@ -147,10 +147,7 @@ def __init__( self._sinkpoints_spec = { "definitions_module": "ddtrace.appsec._iast.taint_sinks", "alias_module": "ddtrace_taint_sinks", - "functions": { - # FIXME: disabled to unblock release 2.9 - # "open": "ddtrace_taint_sinks.open_path_traversal", - }, + "functions": {}, } self._sinkpoints_functions = self._sinkpoints_spec["functions"] diff --git a/ddtrace/appsec/_iast/_patch_modules.py b/ddtrace/appsec/_iast/_patch_modules.py index 5c38f3d0f62..3d7e1b4f50a 100644 --- a/ddtrace/appsec/_iast/_patch_modules.py +++ b/ddtrace/appsec/_iast/_patch_modules.py @@ -4,7 +4,6 @@ IAST_PATCH = { "command_injection": True, "header_injection": True, - "path_traversal": True, "weak_cipher": True, "weak_hash": True, } diff --git a/ddtrace/appsec/_iast/taint_sinks/__init__.py b/ddtrace/appsec/_iast/taint_sinks/__init__.py index e7c8787c943..18196607fc8 100644 --- a/ddtrace/appsec/_iast/taint_sinks/__init__.py +++ b/ddtrace/appsec/_iast/taint_sinks/__init__.py @@ -1,8 +1,6 @@ from .ast_taint import ast_function -from .path_traversal import open_path_traversal __all__ = [ - "open_path_traversal", "ast_function", ] diff --git a/ddtrace/appsec/_iast/taint_sinks/path_traversal.py b/ddtrace/appsec/_iast/taint_sinks/path_traversal.py index 1c51eb273cf..25750b3bb5e 100644 --- a/ddtrace/appsec/_iast/taint_sinks/path_traversal.py +++ b/ddtrace/appsec/_iast/taint_sinks/path_traversal.py @@ -1,14 +1,10 @@ from typing import Any -from typing import Text from ddtrace.internal.logger import get_logger from ..._constants import IAST_SPAN_TAGS from .. import oce -from .._metrics import _set_metric_iast_instrumented_sink from .._metrics import increment_iast_span_metric -from .._patch import set_and_check_module_is_patched -from .._patch import set_module_unpatched from ..constants import VULN_PATH_TRAVERSAL from ..processor import AppSecIastSpanProcessor from ._base import VulnerabilityBase @@ -22,21 +18,6 @@ class PathTraversal(VulnerabilityBase): vulnerability_type = VULN_PATH_TRAVERSAL -def get_version() -> Text: - return "" - - -def unpatch_iast(): - set_module_unpatched("builtins", default_attr="_datadog_path_traversal_patch") - - -def patch(): - """Wrap functions which interact with file system.""" - if not set_and_check_module_is_patched("builtins", default_attr="_datadog_path_traversal_patch"): - return - _set_metric_iast_instrumented_sink(VULN_PATH_TRAVERSAL) - - def check_and_report_path_traversal(*args: Any, **kwargs: Any) -> None: if AppSecIastSpanProcessor.is_span_analyzed() and PathTraversal.has_quota(): try: @@ -45,14 +26,10 @@ def check_and_report_path_traversal(*args: Any, **kwargs: Any) -> None: increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, PathTraversal.vulnerability_type) _set_metric_iast_executed_sink(PathTraversal.vulnerability_type) - if is_pyobject_tainted(args[0]): - PathTraversal.report(evidence_value=args[0]) + filename_arg = args[0] if args else kwargs.get("file", None) + if is_pyobject_tainted(filename_arg): + PathTraversal.report(evidence_value=filename_arg) except Exception: log.debug("Unexpected exception while reporting vulnerability", exc_info=True) else: log.debug("IAST: no vulnerability quota to analyze more sink points") - - -def open_path_traversal(*args, **kwargs): - check_and_report_path_traversal(*args, **kwargs) - return open(*args, **kwargs) diff --git a/tests/appsec/iast/conftest.py b/tests/appsec/iast/conftest.py index fb2da25326b..03512871a2d 100644 --- a/tests/appsec/iast/conftest.py +++ b/tests/appsec/iast/conftest.py @@ -3,6 +3,8 @@ import pytest +from ddtrace.appsec._common_module_patches import patch_common_modules +from ddtrace.appsec._common_module_patches import unpatch_common_modules from ddtrace.appsec._constants import IAST from ddtrace.appsec._iast import oce from ddtrace.appsec._iast._patches.json_tainting import patch as json_patch @@ -13,7 +15,6 @@ from ddtrace.appsec._iast.taint_sinks.command_injection import unpatch as cmdi_unpatch from ddtrace.appsec._iast.taint_sinks.header_injection import patch as header_injection_patch from ddtrace.appsec._iast.taint_sinks.header_injection import unpatch as header_injection_unpatch -from ddtrace.appsec._iast.taint_sinks.path_traversal import patch as path_traversal_patch from ddtrace.appsec._iast.taint_sinks.weak_cipher import patch as weak_cipher_patch from ddtrace.appsec._iast.taint_sinks.weak_cipher import unpatch_iast as weak_cipher_unpatch from ddtrace.appsec._iast.taint_sinks.weak_hash import patch as weak_hash_patch @@ -70,7 +71,6 @@ def iast_span(tracer, env, request_sampling="100", deduplication=False): span.span_type = "web" weak_hash_patch() weak_cipher_patch() - path_traversal_patch() sqli_sqlite_patch() json_patch() psycopg_patch() @@ -79,7 +79,9 @@ def iast_span(tracer, env, request_sampling="100", deduplication=False): header_injection_patch() langchain_patch() iast_span_processor.on_span_start(span) + patch_common_modules() yield span + unpatch_common_modules() iast_span_processor.on_span_finish(span) weak_hash_unpatch() weak_cipher_unpatch() diff --git a/tests/appsec/iast/taint_sinks/test_path_traversal.py b/tests/appsec/iast/taint_sinks/test_path_traversal.py index 5f5abaaa9c9..f7086a0cd59 100644 --- a/tests/appsec/iast/taint_sinks/test_path_traversal.py +++ b/tests/appsec/iast/taint_sinks/test_path_traversal.py @@ -1,5 +1,6 @@ import os +import mock import pytest from ddtrace.appsec._constants import IAST @@ -23,8 +24,6 @@ def _get_path_traversal_module_functions(): yield module, function -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip def test_path_traversal_open(iast_span_defaults): mod = _iast_patched_module("tests.appsec.iast.fixtures.taint_sinks.path_traversal") @@ -51,8 +50,41 @@ def test_path_traversal_open(iast_span_defaults): assert vulnerability["evidence"].get("redacted") is None -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip +@mock.patch("tests.appsec.iast.fixtures.taint_sinks.path_traversal.open") +def test_path_traversal_open_and_mock(mock_open, iast_span_defaults): + """Confirm we can mock the open function and IAST path traversal vulnerability is not reported""" + mod = _iast_patched_module("tests.appsec.iast.fixtures.taint_sinks.path_traversal") + + file_path = os.path.join(ROOT_DIR, "../fixtures", "taint_sinks", "path_traversal_test_file.txt") + + tainted_string = taint_pyobject( + file_path, source_name="path", source_value=file_path, source_origin=OriginType.PATH + ) + mod.pt_open(tainted_string) + + mock_open.assert_called_once_with(file_path) + + span_report = core.get_item(IAST.CONTEXT_KEY, span=iast_span_defaults) + assert span_report is None + + +def test_path_traversal_open_and_mock_after_patch_module(iast_span_defaults): + """Confirm we can mock the open function and IAST path traversal vulnerability is not reported""" + mod = _iast_patched_module("tests.appsec.iast.fixtures.taint_sinks.path_traversal") + with mock.patch("tests.appsec.iast.fixtures.taint_sinks.path_traversal.open") as mock_open: + file_path = os.path.join(ROOT_DIR, "../fixtures", "taint_sinks", "path_traversal_test_file.txt") + + tainted_string = taint_pyobject( + file_path, source_name="path", source_value=file_path, source_origin=OriginType.PATH + ) + mod.pt_open(tainted_string) + + mock_open.assert_called_once_with(file_path) + + span_report = core.get_item(IAST.CONTEXT_KEY, span=iast_span_defaults) + assert span_report is None + + @pytest.mark.parametrize( "file_path", ( @@ -74,8 +106,6 @@ def test_path_traversal_open_secure(file_path, iast_span_defaults): assert span_report is None -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip @pytest.mark.parametrize( "module, function", _get_path_traversal_module_functions(), @@ -109,8 +139,6 @@ def test_path_traversal(module, function, iast_span_defaults): assert vulnerability["evidence"].get("redacted") is None -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip @pytest.mark.parametrize("num_vuln_expected", [1, 0, 0]) def test_path_traversal_deduplication(num_vuln_expected, iast_span_deduplication_enabled): mod = _iast_patched_module("tests.appsec.iast.fixtures.taint_sinks.path_traversal") diff --git a/tests/appsec/iast/test_iast_propagation_path.py b/tests/appsec/iast/test_iast_propagation_path.py index f683b262eb1..9637b692501 100644 --- a/tests/appsec/iast/test_iast_propagation_path.py +++ b/tests/appsec/iast/test_iast_propagation_path.py @@ -27,8 +27,6 @@ def _assert_vulnerability(data, value_parts, file_line_label): assert vulnerability["hash"] == hash_value -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip def test_propagation_no_path(iast_span_defaults): mod = _iast_patched_module("tests.appsec.iast.fixtures.propagation_path") origin1 = "taintsource" @@ -41,8 +39,6 @@ def test_propagation_no_path(iast_span_defaults): assert span_report is None -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip @pytest.mark.parametrize( "origin1", [ @@ -77,8 +73,6 @@ def test_propagation_path_1_origin_1_propagation(origin1, iast_span_defaults): _assert_vulnerability(data, value_parts, "propagation_path_1_source_1_prop") -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip @pytest.mark.parametrize( "origin1", [ @@ -115,8 +109,6 @@ def test_propagation_path_1_origins_2_propagations(origin1, iast_span_defaults): _assert_vulnerability(data, value_parts, "propagation_path_1_source_2_prop") -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip @pytest.mark.parametrize( "origin1, origin2", [ @@ -167,8 +159,6 @@ def test_propagation_path_2_origins_2_propagations(origin1, origin2, iast_span_d _assert_vulnerability(data, value_parts, "propagation_path_2_source_2_prop") -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip @pytest.mark.parametrize( "origin1, origin2", [ @@ -227,8 +217,6 @@ def test_propagation_path_2_origins_3_propagation(origin1, origin2, iast_span_de _assert_vulnerability(data, value_parts, "propagation_path_3_prop") -# FIXME: enable once the mock + open issue is fixed -@pytest.mark.skip @pytest.mark.parametrize( "origin1, origin2", [ diff --git a/tests/appsec/iast/test_telemetry.py b/tests/appsec/iast/test_telemetry.py index 2dfe1f11026..ac49a2700fb 100644 --- a/tests/appsec/iast/test_telemetry.py +++ b/tests/appsec/iast/test_telemetry.py @@ -1,6 +1,8 @@ import pytest from ddtrace.appsec import _asm_request_context +from ddtrace.appsec._common_module_patches import patch_common_modules +from ddtrace.appsec._common_module_patches import unpatch_common_modules from ddtrace.appsec._constants import IAST_SPAN_TAGS from ddtrace.appsec._handlers import _on_django_patch from ddtrace.appsec._iast._metrics import TELEMETRY_DEBUG_VERBOSITY @@ -19,8 +21,6 @@ from ddtrace.appsec._iast.taint_sinks.command_injection import patch as cmdi_patch from ddtrace.appsec._iast.taint_sinks.header_injection import patch as header_injection_patch from ddtrace.appsec._iast.taint_sinks.header_injection import unpatch as header_injection_unpatch -from ddtrace.appsec._iast.taint_sinks.path_traversal import patch as path_traversal_patch -from ddtrace.appsec._iast.taint_sinks.path_traversal import unpatch_iast as path_traversal_unpatch from ddtrace.contrib.sqlalchemy import patch as sqli_sqlalchemy_patch from ddtrace.contrib.sqlite3 import patch as sqli_sqlite3_patch from ddtrace.ext import SpanTypes @@ -107,11 +107,11 @@ def test_metric_instrumented_cmdi(no_request_sampling, telemetry_writer): def test_metric_instrumented_path_traversal(no_request_sampling, telemetry_writer): # We need to unpatch first because ddtrace.appsec._iast._patch_modules loads at runtime this patch function - path_traversal_unpatch() + unpatch_common_modules() with override_env(dict(DD_IAST_TELEMETRY_VERBOSITY="INFORMATION")), override_global_config( dict(_iast_enabled=True) ): - path_traversal_patch() + patch_common_modules() _assert_instrumented_sink(telemetry_writer, VULN_PATH_TRAVERSAL)