Skip to content

Commit 1385ba6

Browse files
fix(iast): avoid patching errors from stopping module load [backport 2.5] (#8546)
Backport 476ecb0 from #8527 to 2.5. IAST: This fix addresses an issue where AST patching would generate code that fails to compile, thereby preventing the application from starting correctly. While the root cause is not addressed here, this PR ensures that if the produced code fails to compile, we would load the original module instead. ## 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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [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)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. - [x] If change 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`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has 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) Co-authored-by: Federico Mon <[email protected]>
1 parent c91054d commit 1385ba6

File tree

4 files changed

+57
-1
lines changed

4 files changed

+57
-1
lines changed

ddtrace/appsec/_iast/_loader.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
def _exec_iast_patched_module(module_watchdog, module):
1616
patched_source = None
17+
compiled_code = None
1718
if IS_IAST_ENABLED:
1819
try:
1920
module_path, patched_source = astpatch_module(module)
@@ -22,8 +23,15 @@ def _exec_iast_patched_module(module_watchdog, module):
2223
patched_source = None
2324

2425
if patched_source:
26+
try:
27+
# Patched source is compiled in order to execute it
28+
compiled_code = compile(patched_source, module_path, "exec")
29+
except Exception:
30+
log.debug("Unexpected exception while compiling patched code", exc_info=True)
31+
compiled_code = None
32+
33+
if compiled_code:
2534
# Patched source is executed instead of original module
26-
compiled_code = compile(patched_source, module_path, "exec")
2735
exec(compiled_code, module.__dict__) # nosec B102
2836
elif module_watchdog.loader is not None:
2937
module_watchdog.loader.exec_module(module)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
Vulnerability Management for Code-level (IAST): This fix addresses an issue where AST patching would generate code that fails to compile, thereby preventing the application from starting correctly.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#!/usr/bin/env python3
2+
3+
4+
def add(a, b):
5+
return a + b

tests/appsec/iast/test_loader.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
#!/usr/bin/env python3
2+
3+
import importlib
4+
import sys
5+
6+
import mock
7+
8+
import ddtrace.appsec._iast._loader
9+
import ddtrace.bootstrap.preload
10+
from tests.utils import override_env
11+
12+
13+
ASPECTS_MODULE = "ddtrace.appsec._iast._taint_tracking.aspects"
14+
15+
16+
def test_patching_error():
17+
"""
18+
When IAST is enabled and the loader fails to compile the module,
19+
the module should still be imported successfully.
20+
"""
21+
fixture_module = "tests.appsec.iast.fixtures.loader"
22+
if fixture_module in sys.modules:
23+
del sys.modules[fixture_module]
24+
25+
if ASPECTS_MODULE in sys.modules:
26+
del sys.modules[ASPECTS_MODULE]
27+
28+
ddtrace.appsec._iast._loader.IS_IAST_ENABLED = True
29+
30+
with override_env({"DD_IAST_ENABLED": "true"}), mock.patch(
31+
"ddtrace.appsec._iast._loader.compile", side_effect=ValueError
32+
) as loader_compile, mock.patch("ddtrace.appsec._iast._loader.exec") as loader_exec:
33+
importlib.reload(ddtrace.bootstrap.preload)
34+
imported_fixture_module = importlib.import_module(fixture_module)
35+
36+
imported_fixture_module.add(2, 1)
37+
loader_compile.assert_called_once()
38+
loader_exec.assert_not_called()
39+
assert ASPECTS_MODULE not in sys.modules

0 commit comments

Comments
 (0)