Skip to content

Commit 5b4ad29

Browse files
fix(iast): ast patching error in mysqlsh [backport 3.10] (#14002)
Backport 46f013d from #13979 to 3.10. The AST analysis could yield unexpected or incorrect results when analyzingcode that overwrites built-in or global names at runtime. A notable example is `mysqlsh` (MySQL Shell), which reassigns `globals` with something like: `globals = ShellGlobals()`. Since `globals` is a built-in function in Python, reassigning it alters the global namespace's behavior during analysis. This can cause dynamic instrumentation, taint tracking, or symbol resolution to behave incorrectly or inconsistently How to test manually, create this Dockerfile ``` FROM python:3.13-slim RUN apt-get update && \ apt-get install -y gnupg wget lsb-release && \ wget https://dev.mysql.com/get/mysql-apt-config_0.8.29-1_all.deb && \ DEBIAN_FRONTEND=noninteractive dpkg -i mysql-apt-config_0.8.29-1_all.deb && \ apt-get update && \ apt-get install -y mysql-shell vim && \ rm -f mysql-apt-config_0.8.29-1_all.deb && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* RUN pip install bytecode wrapt envier ADD . /app WORKDIR /app ``` and execute: ``` docker build -t python-mysqlsh . docker run -it --rm python-mysqlsh bash >>> export PATH=$PATH:$PWD && export PYTHONPATH=$PYTHONPATH:$PWD >>> DD_TRACE_DEBUG=true DD_IAST_ENABLED=true python -m ddtrace.commands.ddtrace_run mysqlsh ``` ## 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) Co-authored-by: Alberto Vara <[email protected]>
1 parent b053ce8 commit 5b4ad29

File tree

5 files changed

+72
-4
lines changed

5 files changed

+72
-4
lines changed

ddtrace/appsec/_iast/_ast/iastpatch.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ static size_t cached_packages_count = 0;
2020
static const char* static_allowlist[] = {
2121
"jinja2.", "pygments.", "multipart.", "sqlalchemy.", "python_multipart.",
2222
"attrs.", "jsonschema.", "s3fs.", "mysql.", "pymysql.",
23-
"markupsafe.", "werkzeug.utils.", "langchain.", "langchain_core.", "django.http.response"
23+
"markupsafe.", "werkzeug.utils.", "langchain.", "langchain_core.", "django.http.response."
2424
};
2525
static const size_t static_allowlist_count = sizeof(static_allowlist) / sizeof(static_allowlist[0]);
2626

@@ -170,6 +170,7 @@ static const char* static_denylist[] = {
170170
"django_filters.rest_framework.filterset.",
171171
"django_filters.utils.",
172172
"django_filters.widgets.",
173+
"mysqlsh.",
173174
};
174175
static const size_t static_denylist_count = sizeof(static_denylist) / sizeof(static_denylist[0]);
175176

ddtrace/appsec/_iast/_loader.py

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,32 @@
1111

1212

1313
def _exec_iast_patched_module(module_watchdog, module):
14+
"""Execute a Python module with IAST (Interactive Application Security Testing) instrumentation.
15+
16+
This function performs dynamic code transformation using AST (Abstract Syntax Tree) patching
17+
to inject security vulnerability detection capabilities into Python modules at runtime.
18+
It's a core component of the IAST engine that enables taint tracking and vulnerability
19+
detection without modifying the original source code.
20+
21+
How it works:
22+
1. Attempts to patch the module's AST using astpatch_module()
23+
2. If successful, compiles the patched AST into executable bytecode
24+
3. Executes the instrumented bytecode instead of the original module
25+
4. Falls back to executing the original module if patching fails
26+
27+
Runtime Considerations:
28+
- The AST analysis could yield unexpected or incorrect results when analyzing
29+
code that overwrites built-in or global names at runtime
30+
- A notable example is `mysqlsh` (MySQL Shell), which reassigns `globals` with
31+
something like: `globals = ShellGlobals()`. Since `globals` is a built-in
32+
function in Python, reassigning it alters the global namespace's behavior
33+
during analysis. This can cause dynamic instrumentation, taint tracking,
34+
or symbol resolution to behave incorrectly or inconsistently
35+
- The function gracefully handles compilation and execution errors by falling
36+
back to the original module execution
37+
- All exceptions during patching are logged but don't prevent module execution
38+
"""
39+
1440
patched_ast = None
1541
compiled_code = None
1642
if IS_IAST_ENABLED:
@@ -19,7 +45,6 @@ def _exec_iast_patched_module(module_watchdog, module):
1945
except Exception:
2046
iast_compiling_debug_log("Unexpected exception while AST patching", exc_info=True)
2147
patched_ast = None
22-
2348
if patched_ast:
2449
try:
2550
# Patched source is compiled in order to execute it
@@ -30,8 +55,12 @@ def _exec_iast_patched_module(module_watchdog, module):
3055

3156
if compiled_code:
3257
iast_compiling_debug_log(f"INSTRUMENTED CODE. executing {module_path}")
33-
# Patched source is executed instead of original module
34-
exec(compiled_code, module.__dict__) # nosec B102
58+
try:
59+
# Patched source is executed instead of original module
60+
exec(compiled_code, module.__dict__) # nosec B102
61+
except TypeError:
62+
iast_compiling_debug_log("INSTRUMENTED CODE. Unexpected exception", exc_info=True)
63+
module_watchdog.loader.exec_module(module)
3564
elif module_watchdog.loader is not None:
3665
try:
3766
iast_compiling_debug_log(f"DEFAULT CODE. executing {module}")
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- |
4+
Code Security: AST analysis may fail or behave unexpectedly in cases where code overrides Python built-ins or
5+
globals at runtime, e.g., ``mysqlsh`` (MySQL Shell) reassigns globals with a custom object.
6+
This can interfere with analysis or instrumentation logic.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
class ShellGlobals(object):
2+
def __setattr__(self, name, value):
3+
self.__dict__[name] = value
4+
5+
def __delattr__(self, name):
6+
del self.__dict__[name]
7+
8+
def __getattr__(self, name):
9+
# backwards compatibility
10+
if name == "mysql":
11+
return "mysql"
12+
elif name == "mysqlx":
13+
return "mysqlx"
14+
return self.__dict__[name]
15+
16+
17+
globals = ShellGlobals() # noqa: A001
18+
19+
del ShellGlobals
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import sys
2+
3+
import pytest
4+
5+
from tests.appsec.iast.iast_utils import _iast_patched_module
6+
7+
8+
@pytest.mark.skipif(sys.version_info < (3, 9), reason="This test doesn't raises an error on 3.8")
9+
def test_string_proper_method_called():
10+
"""Fixed in ddtrace/appsec/_iast/_loader.py except TypeError:"""
11+
with pytest.raises(TypeError):
12+
mod = _iast_patched_module("tests.appsec.iast._ast.fixtures.del_variables_at_runtime")
13+
assert mod

0 commit comments

Comments
 (0)