Skip to content

Commit 69249c2

Browse files
authored
chore(iast): split wrapped_view hook [backport 2.21] (#12726)
Backport 8670015 from #12706 to 2.21. `_on_wrapped_view` contains the logic to block requests for AppSec and to taint path parameters for IAST. However, since the `_on_wrapped_view` hook function is in `load_appsec`, the IAST logic doesn’t run if AppSec isn’t enabled. This PR splits that logic and creates two separate hooks. This PR is a cherry-pick of one of the commits of this PR #12639 Backport #12726 to fix the CI ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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)
1 parent 6cc1ecc commit 69249c2

File tree

12 files changed

+57
-56
lines changed

12 files changed

+57
-56
lines changed

.gitlab/tests.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ build_base_venvs:
6464
matrix:
6565
- PYTHON_VERSION: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]
6666
variables:
67-
CMAKE_BUILD_PARALLEL_LEVEL: 12
6867
PIP_VERBOSE: 1
6968
DD_PROFILING_NATIVE_TESTS: 1
69+
DD_FAST_BUILD: 1
7070
script:
7171
- pip install riot==0.20.1
7272
- riot -P -v generate --python=$PYTHON_VERSION

ddtrace/appsec/_asm_request_context.py

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,6 @@
2525
from ddtrace.trace import Span
2626

2727

28-
if asm_config._iast_enabled:
29-
from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled
30-
from ddtrace.appsec._iast._taint_tracking import OriginType
31-
from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject
32-
else:
33-
34-
def is_iast_request_enabled() -> bool:
35-
return False
36-
37-
3828
if TYPE_CHECKING:
3929
from ddtrace.appsec._ddwaf import DDWaf_info
4030
from ddtrace.appsec._ddwaf import DDWaf_result
@@ -486,7 +476,7 @@ def _on_context_ended(ctx):
486476

487477

488478
def _on_wrapped_view(kwargs):
489-
return_value = [None, None]
479+
callback_block = None
490480
# if Appsec is enabled, we can try to block as we have the path parameters at that point
491481
if asm_config._asm_enabled and in_asm_context():
492482
log.debug("Flask WAF call for Suspicious Request Blocking on request")
@@ -495,21 +485,7 @@ def _on_wrapped_view(kwargs):
495485
call_waf_callback()
496486
if is_blocked():
497487
callback_block = get_value(_CALLBACKS, "flask_block")
498-
return_value[0] = callback_block
499-
500-
# If IAST is enabled, taint the Flask function kwargs (path parameters)
501-
502-
if asm_config._iast_enabled and kwargs:
503-
if not is_iast_request_enabled():
504-
return return_value
505-
506-
_kwargs = {}
507-
for k, v in kwargs.items():
508-
_kwargs[k] = taint_pyobject(
509-
pyobject=v, source_name=k, source_value=v, source_origin=OriginType.PATH_PARAMETER
510-
)
511-
return_value[1] = _kwargs
512-
return return_value
488+
return callback_block
513489

514490

515491
def _on_pre_tracedrequest(ctx):
@@ -574,7 +550,7 @@ def asm_listen():
574550
listen()
575551

576552
core.on("flask.finalize_request.post", _set_headers_and_response)
577-
core.on("flask.wrapped_view", _on_wrapped_view, "callback_and_args")
553+
core.on("flask.wrapped_view", _on_wrapped_view, "callbacks")
578554
core.on("flask._patched_request", _on_pre_tracedrequest)
579555
core.on("wsgi.block_decided", _on_block_decided)
580556
core.on("flask.start_response", _call_waf_first, "waf")

ddtrace/appsec/_iast/_handlers.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,18 @@ def _on_flask_patch(flask_version):
134134
_set_metric_iast_instrumented_source(OriginType.PARAMETER_NAME)
135135

136136

137+
def _iast_on_wrapped_view(kwargs):
138+
# If IAST is enabled, taint the Flask function kwargs (path parameters)
139+
if kwargs and asm_config._iast_enabled:
140+
_kwargs = {}
141+
for k, v in kwargs.items():
142+
_kwargs[k] = taint_pyobject(
143+
pyobject=v, source_name=k, source_value=v, source_origin=OriginType.PATH_PARAMETER
144+
)
145+
return _kwargs
146+
return kwargs
147+
148+
137149
def _on_wsgi_environ(wrapped, _instance, args, kwargs):
138150
if asm_config._iast_enabled and args and is_iast_request_enabled():
139151
return wrapped(*((taint_structure(args[0], OriginType.HEADER_NAME, OriginType.HEADER),) + args[1:]), **kwargs)
@@ -161,8 +173,9 @@ def _on_django_patch():
161173
functools.partial(if_iast_taint_returned_object_for, OriginType.PARAMETER),
162174
)
163175
)
176+
log.debug("[IAST] Patching Django correctly")
164177
except Exception:
165-
log.debug("Unexpected exception while patch IAST functions", exc_info=True)
178+
log.debug("[IAST] Unexpected exception while patch Django", exc_info=True)
166179

167180

168181
def _on_django_func_wrapped(fn_args, fn_kwargs, first_arg_expected_type, *_):

ddtrace/appsec/_iast/_listener.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from ddtrace.appsec._iast._handlers import _iast_on_wrapped_view
12
from ddtrace.appsec._iast._handlers import _on_django_func_wrapped
23
from ddtrace.appsec._iast._handlers import _on_django_patch
34
from ddtrace.appsec._iast._handlers import _on_flask_patch
@@ -21,8 +22,9 @@ def iast_listen():
2122
core.on("django.func.wrapped", _on_django_func_wrapped)
2223
core.on("flask.patch", _on_flask_patch)
2324
core.on("flask.request_init", _on_request_init)
24-
core.on("flask._patched_request", _on_pre_tracedrequest_iast)
2525
core.on("flask.set_request_tags", _on_set_request_tags_iast)
26+
core.on("flask.wrapped_view", _iast_on_wrapped_view, "check_kwargs")
27+
core.on("flask._patched_request", _on_pre_tracedrequest_iast)
2628

2729
core.on("context.ended.wsgi.__call__", _iast_end_request)
2830
core.on("context.ended.asgi.__call__", _iast_end_request)

ddtrace/contrib/internal/flask/wrappers.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,22 @@ def _wrap_call(
4545
tags=tags,
4646
) as ctx, ctx.span:
4747
if do_dispatch:
48-
result = core.dispatch_with_results("flask.wrapped_view", (kwargs,)).callback_and_args
48+
dispatch = core.dispatch_with_results("flask.wrapped_view", (kwargs,))
49+
50+
# Appsec blocks the request
51+
result = dispatch.callbacks
4952
if result:
50-
callback_block, _kwargs = result.value
53+
callback_block = result.value
5154
if callback_block:
5255
return callback_block()
56+
# IAST overrides the kwargs
57+
result = dispatch.check_kwargs
58+
if result:
59+
_kwargs = result.value
5360
if _kwargs:
5461
for k in kwargs:
5562
kwargs[k] = _kwargs[k]
63+
5664
return wrapped(*args, **kwargs)
5765

5866

tests/appsec/appsec/test_telemetry.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import os
22
from time import sleep
3+
from unittest import mock
34

4-
import mock
55
import pytest
66

77
import ddtrace.appsec._asm_request_context as asm_request_context

tests/appsec/iast/conftest.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,13 @@ def iast_span_defaults(tracer):
138138
yield span
139139

140140

141-
# The log contains "[IAST]" but "[IAST] create_context" or "[IAST] reset_context" are valid
142-
IAST_VALID_LOG = re.compile(r"(?=.*\[IAST\] )(?!.*\[IAST\] (create_context|reset_context))")
141+
# Check if the log contains "[IAST]" to raise an error if that’s the case BUT, if the logs contains
142+
# "[IAST] create_context", "[IAST] reset_context", "[IAST] allowing", "[IAST] denying" or "[IAST] astpatch_source"
143+
# are valid logs
144+
IAST_VALID_LOG = re.compile(
145+
r"(?=.*\[IAST\] )(?!.*\[IAST\] "
146+
r"(Patching|Enabled|astpatch_source|compile_code|allowing|denying|create_context|reset_context))"
147+
)
143148

144149

145150
@pytest.fixture(autouse=True)

tests/contrib/flask/test_flask_appsec_iast.py renamed to tests/appsec/integrations/flask_tests/test_iast_flask.py

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from ddtrace.appsec._iast import oce
1010
from ddtrace.appsec._iast._iast_request_context import _iast_start_request
1111
from ddtrace.appsec._iast._patches.json_tainting import patch as patch_json
12+
from ddtrace.appsec._iast._taint_tracking._taint_objects import is_pyobject_tainted
1213
from ddtrace.appsec._iast.constants import VULN_HEADER_INJECTION
1314
from ddtrace.appsec._iast.constants import VULN_INSECURE_COOKIE
1415
from ddtrace.appsec._iast.constants import VULN_NO_HTTPONLY_COOKIE
@@ -19,12 +20,10 @@
1920
from ddtrace.settings.asm import config as asm_config
2021
from tests.appsec.iast.iast_utils import get_line_and_hash
2122
from tests.contrib.flask import BaseFlaskTestCase
22-
from tests.utils import flaky
23-
from tests.utils import override_env
2423
from tests.utils import override_global_config
2524

2625

27-
TEST_FILE_PATH = "tests/contrib/flask/test_flask_appsec_iast.py"
26+
TEST_FILE_PATH = "tests/appsec/integrations/flask_tests/test_iast_flask.py"
2827

2928
werkzeug_version = version("werkzeug")
3029
flask_version = tuple([int(v) for v in version("flask").split(".")])
@@ -37,7 +36,7 @@ def inject_fixtures(self, caplog, telemetry_writer): # noqa: F811
3736
self._caplog = caplog
3837

3938
def setUp(self):
40-
with override_env({"_DD_IAST_USE_ROOT_SPAN": "false"}), override_global_config(
39+
with override_global_config(
4140
dict(
4241
_iast_enabled=True,
4342
_iast_deduplication_enabled=False,
@@ -49,10 +48,9 @@ def setUp(self):
4948
patch_header_injection()
5049
patch_json()
5150

52-
self.tracer._configure(api_version="v0.4", appsec_enabled=True, iast_enabled=True)
51+
self.tracer._configure(api_version="v0.4", iast_enabled=True)
5352
oce.reconfigure()
5453

55-
@flaky(1735812000)
5654
@pytest.mark.skipif(not asm_config._iast_supported, reason="Python version not supported by IAST")
5755
def test_flask_full_sqli_iast_http_request_path_parameter(self):
5856
@self.app.route("/sqli/<string:param_str>/", methods=["GET", "POST"])
@@ -1462,8 +1460,6 @@ def cookie_secure():
14621460
from flask import Response
14631461
from flask import request
14641462

1465-
from ddtrace.appsec._iast._taint_tracking._taint_objects import is_pyobject_tainted
1466-
14671463
tainted_string = request.form.get("name")
14681464
assert is_pyobject_tainted(tainted_string)
14691465
resp = Response("OK")
@@ -1636,8 +1632,6 @@ def test_flask_simple_iast_path_header_and_querystring_not_tainted_if_iast_disab
16361632
def test_sqli(param_str):
16371633
from flask import request
16381634

1639-
from ddtrace.appsec._iast._taint_tracking._taint_objects import is_pyobject_tainted
1640-
16411635
assert not is_pyobject_tainted(request.headers["User-Agent"])
16421636
assert not is_pyobject_tainted(request.query_string)
16431637
assert not is_pyobject_tainted(param_str)

0 commit comments

Comments
 (0)