Skip to content

Commit fd5c86c

Browse files
fix(iast): avoid crashing grpc patch if MessageMapContainer import fails [backport 2.9] (#9219)
Backport b2840ab from #9215 to 2.9. Code Security: Fix an issue at GRPC patching where if `google._upb` module wasn't available, an unhandled ImportError would be raised. ## 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`. ## 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 4f059c8 commit fd5c86c

File tree

3 files changed

+38
-9
lines changed

3 files changed

+38
-9
lines changed

ddtrace/appsec/_handlers.py

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from collections.abc import MutableMapping
12
import functools
23
import io
34
import json
@@ -19,6 +20,13 @@
1920
from ddtrace.vendor.wrapt import wrap_function_wrapper as _w
2021

2122

23+
MessageMapContainer = None
24+
try:
25+
from google._upb._message import MessageMapContainer # type: ignore[no-redef]
26+
except ImportError:
27+
pass
28+
29+
2230
log = get_logger(__name__)
2331
_BODY_METHODS = {"POST", "PUT", "DELETE", "PATCH"}
2432

@@ -359,10 +367,6 @@ def _on_django_patch():
359367

360368

361369
def _custom_protobuf_getattribute(self, name):
362-
from collections.abc import MutableMapping
363-
364-
from google._upb._message import MessageMapContainer
365-
366370
from ddtrace.appsec._iast._taint_tracking import taint_pyobject
367371
from ddtrace.appsec._iast._taint_tracking._native.taint_tracking import OriginType
368372
from ddtrace.appsec._iast._taint_utils import taint_structure
@@ -375,7 +379,7 @@ def _custom_protobuf_getattribute(self, name):
375379
source_value=ret,
376380
source_origin=OriginType.GRPC_BODY,
377381
)
378-
elif isinstance(ret, MutableMapping):
382+
elif MessageMapContainer is not None and isinstance(ret, MutableMapping):
379383
if isinstance(ret, MessageMapContainer) and len(ret):
380384
# Patch the message-values class
381385
first_key = next(iter(ret))
@@ -398,10 +402,14 @@ def _patch_protobuf_class(cls):
398402
return
399403

400404
if not hasattr(getattr_method, "__datadog_custom"):
401-
# Replace the class __getattribute__ method with our custom one
402-
# (replacement is done at the class level because it would incur on a recursive loop with the instance)
403-
cls.__saved_getattr = getattr_method
404-
cls.__getattribute__ = _custom_protobuf_getattribute
405+
try:
406+
# Replace the class __getattribute__ method with our custom one
407+
# (replacement is done at the class level because it would incur on a recursive loop with the instance)
408+
cls.__saved_getattr = getattr_method
409+
cls.__getattribute__ = _custom_protobuf_getattribute
410+
except TypeError:
411+
# Avoid failing on Python 3.12 while patching immutable types
412+
pass
405413

406414

407415
def _on_grpc_response(response):
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
Code Security: Avoid an ``ImportError`` at patching stage if ``google._upb`` module is not available.

tests/appsec/iast/test_grpc_iast.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import grpc
22
from grpc._grpcio_metadata import __version__ as _GRPC_VERSION
3+
import mock
34

45
from tests.contrib.grpc.common import GrpcBaseTestCase
56
from tests.contrib.grpc.hello_pb2 import HelloRequest
@@ -72,3 +73,19 @@ def test_taint_iast_last(self):
7273
res = stub1.SayHelloLast(requests_iterator)
7374
assert hasattr(res, "message")
7475
_check_test_range(res.message)
76+
77+
def test_taint_iast_patching_import_error(self):
78+
with mock.patch.dict("sys.modules", {"google._upb._message": None}), override_env({"DD_IAST_ENABLED": "True"}):
79+
from collections import UserDict
80+
81+
from ddtrace.appsec._handlers import _custom_protobuf_getattribute
82+
from ddtrace.appsec._handlers import _patch_protobuf_class
83+
84+
class MyUserDict(UserDict):
85+
pass
86+
87+
_patch_protobuf_class(MyUserDict)
88+
original_dict = {"apple": 1, "banana": 2}
89+
mutable_mapping = MyUserDict(original_dict)
90+
91+
_custom_protobuf_getattribute(mutable_mapping, "data")

0 commit comments

Comments
 (0)