Skip to content

Commit 5a1622f

Browse files
fix(asm): add __deepcopy__ for wrapped builtin functions [backport 3.3] (#12953)
Backport ff1271b from #12948 to 3.3. ## Description This PR fixes a NotImplementedError that occurred when trying to deepcopy wrapped builtin functions (like `open`) while ASM or IAST were enabled. ### The Problem When ASM/IAST is enabled, we wrap certain builtin functions to add security instrumentation. However, when these wrapped functions were deepcopied, they would raise a NotImplementedError because the wrapper didn't implement the `__deepcopy__` method: ```python import copy # This would raise NotImplementedError before the fix copy.deepcopy(open) # NotImplementedError: object proxy must define __deepcopy__() ``` ### The Solution We've modified the `BuiltinFunctionWrapper` class to properly handle deepcopying by: 1. Implementing `__deepcopy__` to create a new wrapper instance 2. Preserving the original function type through the wrapper 3. Ensuring the wrapped function maintains its original type after deepcopy ### Testing - Added test cases to verify that wrapped builtin functions can be deepcopied without errors - Verified that the wrapper preserves all necessary attributes and functionality - Tested with Python 3.13 to ensure compatibility ### Impact This fix resolves the NotImplementedError when deepcopying wrapped builtin functions, making the ASM/IAST functionality more robust and compatible with code that needs to deepcopy these functions. ## 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 b9dc640 commit 5a1622f

File tree

5 files changed

+168
-1
lines changed

5 files changed

+168
-1
lines changed

ddtrace/appsec/_common_module_patches.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
# This module must not import other modules unconditionally that require iast
2-
32
import ctypes
43
import os
54
from typing import Any
@@ -317,6 +316,7 @@ def wrap_object(module, name, factory, args=(), kwargs=None):
317316
(parent, attribute, original) = resolve_path(module, name)
318317
wrapper = factory(original, *args, **kwargs)
319318
apply_patch(parent, attribute, wrapper)
319+
wrapper.__deepcopy__ = lambda memo: wrapper
320320
return wrapper
321321

322322

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
ASM: Fixed a NotImplementedError that occurred when trying to deepcopy wrapped builtin functions (like `open`)
5+
while ASM or IAST were enabled. The error was caused by the wrapper not implementing the `__deepcopy__` method.

tests/appsec/app.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
""" This Flask application is imported on tests.appsec.appsec_utils.gunicorn_server
22
"""
3+
import copy
34
import os
45
import re
56
import subprocess # nosec
67

78
from flask import Flask
89
from flask import Response
910
from flask import request
11+
from wrapt import FunctionWrapper
1012

1113

1214
import ddtrace.auto # noqa: F401 # isort: skip
@@ -980,6 +982,12 @@ def iast_ast_patching_non_re_search():
980982
return resp
981983

982984

985+
@app.route("/common-modules-patch-read", methods=["GET"])
986+
def test_flask_common_modules_patch_read():
987+
copy_open = copy.deepcopy(open)
988+
return Response(f"OK: {isinstance(copy_open, FunctionWrapper)}")
989+
990+
983991
if __name__ == "__main__":
984992
env_port = os.getenv("FLASK_RUN_PORT", 8000)
985993
debug = asbool(os.getenv("FLASK_DEBUG", "false"))
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
import builtins
2+
import copy
3+
import types
4+
5+
import pytest
6+
from wrapt import FunctionWrapper
7+
8+
from ddtrace.appsec._common_module_patches import patch_common_modules
9+
from ddtrace.appsec._common_module_patches import try_unwrap
10+
from ddtrace.appsec._common_module_patches import try_wrap_function_wrapper
11+
from ddtrace.appsec._common_module_patches import unpatch_common_modules
12+
13+
14+
def test_patch_read():
15+
unpatch_common_modules()
16+
copy_open = copy.deepcopy(open)
17+
18+
assert copy_open is open
19+
assert type(open) == types.BuiltinFunctionType
20+
assert not isinstance(open, FunctionWrapper)
21+
assert not isinstance(copy_open, FunctionWrapper)
22+
assert isinstance(open, types.BuiltinFunctionType)
23+
24+
25+
def test_patch_read_enabled():
26+
unpatch_common_modules()
27+
original_open = open
28+
try:
29+
patch_common_modules()
30+
copy_open = copy.deepcopy(open)
31+
32+
assert type(open) == FunctionWrapper
33+
assert isinstance(copy_open, FunctionWrapper)
34+
assert isinstance(open, FunctionWrapper)
35+
assert hasattr(open, "__wrapped__")
36+
assert open.__wrapped__ is original_open
37+
finally:
38+
unpatch_common_modules()
39+
40+
41+
@pytest.mark.parametrize(
42+
"builtin_function_name",
43+
[
44+
"all",
45+
"any",
46+
"ascii",
47+
"bin",
48+
"bool",
49+
"breakpoint",
50+
"bytearray",
51+
"bytes",
52+
"callable",
53+
"chr",
54+
"classmethod",
55+
"compile",
56+
"complex",
57+
"copyright",
58+
"credits",
59+
"delattr",
60+
"dict",
61+
"dir",
62+
"divmod",
63+
"enumerate",
64+
"eval",
65+
"exec",
66+
"exit",
67+
"filter",
68+
"float",
69+
"format",
70+
"frozenset",
71+
"getattr",
72+
"globals",
73+
"hasattr",
74+
"hash",
75+
"help",
76+
"hex",
77+
"id",
78+
"input",
79+
"int",
80+
"isinstance",
81+
"issubclass",
82+
"iter",
83+
"len",
84+
"license",
85+
"list",
86+
"locals",
87+
"map",
88+
"max",
89+
"memoryview",
90+
"min",
91+
"next",
92+
"object",
93+
"oct",
94+
"open",
95+
"ord",
96+
"pow",
97+
"print",
98+
"property",
99+
"quit",
100+
"range",
101+
"repr",
102+
"reversed",
103+
"round",
104+
"set",
105+
"setattr",
106+
"slice",
107+
"sorted",
108+
"staticmethod",
109+
"str",
110+
"sum",
111+
"super",
112+
"tuple",
113+
"vars",
114+
"zip",
115+
],
116+
)
117+
def test_other_builtin_functions(builtin_function_name):
118+
def dummywrapper(callable, instance, args, kwargs): # noqa: A002
119+
return callable(*args, **kwargs)
120+
121+
try:
122+
try_wrap_function_wrapper("builtins", builtin_function_name, dummywrapper)
123+
124+
original_func = getattr(builtins, builtin_function_name)
125+
copy_func = copy.deepcopy(original_func)
126+
127+
assert type(original_func) == FunctionWrapper
128+
assert isinstance(copy_func, FunctionWrapper)
129+
assert isinstance(original_func, FunctionWrapper)
130+
assert hasattr(original_func, "__wrapped__")
131+
finally:
132+
try_unwrap("builtins", builtin_function_name)

tests/appsec/integrations/flask_tests/test_appsec_flask.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
from ddtrace.contrib.internal.sqlite3.patch import patch
66
from ddtrace.ext import http
77
from ddtrace.internal import constants
8+
from tests.appsec.appsec_utils import flask_server
9+
from tests.appsec.integrations.flask_tests.utils import _PORT
810
import tests.appsec.rules as rules
911
from tests.contrib.flask import BaseFlaskTestCase
1012
from tests.utils import override_global_config
@@ -85,3 +87,23 @@ def test_route(user_id):
8587

8688
resp = self.client.get("/checkuser/%s" % _ALLOWED_USER, headers={"Accept": "text/html"})
8789
assert resp.status_code == 200
90+
91+
92+
@pytest.mark.parametrize(
93+
"iast_enabled",
94+
["true", "false"],
95+
)
96+
@pytest.mark.parametrize("appsec_enabled", ["true", "false"])
97+
def test_flask_common_modules_patch_read(iast_enabled, appsec_enabled):
98+
with flask_server(
99+
appsec_enabled=iast_enabled, iast_enabled=appsec_enabled, token=None, port=_PORT, assert_debug=False
100+
) as context:
101+
_, flask_client, pid = context
102+
103+
response = flask_client.get("/common-modules-patch-read")
104+
105+
assert response.status_code == 200
106+
if iast_enabled == appsec_enabled == "false":
107+
assert response.content == b"OK: False"
108+
else:
109+
assert response.content == b"OK: True"

0 commit comments

Comments
 (0)