Skip to content

Commit 1b69dcb

Browse files
fix(iast): remove native exceptions [backport 2.8] (#8853)
Backport 79f99ef from #8841 to 2.8. Some native exceptions were not being caught correctly by the python tracer. This fix remove those exceptions to avoid fatal error executions like: ``` File "/usr/local/lib/python3.10/dist-packages/ddtrace/appsec/_iast/_taint_tracking/__init__.py", line 106 in taint_pyobject Current thread 0x0000ffff83fbc420 Fatal Python error: g_initialstub: greenlet: Unhandled C++ exception: Tainted Map isn't initialized. Call create_context() first ``` ```python destroy_context() try: result = taint_pyobject( pyobject=string_to_taint, source_name="test_add_aspect_tainting_left_hand", source_value=string_to_taint, source_origin=OriginType.PARAMETER, ) except Exception: pass ``` ``` Fatal Python error: Aborted Thread 0x000078a301cc9700 (most recent call first): File "/home/alberto.vara/.pyenv/versions/3.9.1/lib/python3.9/threading.py", line 316 in wait ``` ## 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: Alberto Vara <[email protected]>
1 parent eb635dd commit 1b69dcb

File tree

3 files changed

+83
-6
lines changed

3 files changed

+83
-6
lines changed

ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintRange.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ set_ranges(PyObject* str, const TaintRangeRefs& ranges, TaintRangeMapType* tx_ma
152152
if (not tx_map) {
153153
tx_map = initializer->get_tainting_map();
154154
if (not tx_map) {
155-
throw py::value_error("Tainted Map isn't initialized. Call create_context() first");
155+
// TODO: Log "Tainted Map isn't initialized. Call create_context() first";
156+
return;
156157
}
157158
}
158159

@@ -246,7 +247,8 @@ get_tainted_object(PyObject* str, TaintRangeMapType* tx_map)
246247
if (not tx_map) {
247248
tx_map = initializer->get_tainting_map();
248249
if (not tx_map) {
249-
throw py::value_error("Tainted Map isn't initialized. Call create_context() first");
250+
// TODO: Log "Tainted Map isn't initialized. Call create_context() first";
251+
return nullptr;
250252
}
251253
}
252254
if (is_notinterned_notfasttainted_unicode(str) or tx_map->empty()) {
@@ -294,7 +296,8 @@ set_tainted_object(PyObject* str, TaintedObjectPtr tainted_object, TaintRangeMap
294296
if (not tx_taint_map) {
295297
tx_taint_map = initializer->get_tainting_map();
296298
if (not tx_taint_map) {
297-
throw py::value_error("Tainted Map isn't initialized. Call create_context() first");
299+
// TODO: Log "Tainted Map isn't initialized. Call create_context() first";
300+
return;
298301
}
299302
}
300303

@@ -347,9 +350,6 @@ pyexport_taintrange(py::module& m)
347350
"parameter_list"_a,
348351
py::return_value_policy::move);
349352

350-
// TODO: check return value policy
351-
m.def("get_tainted_object", &get_tainted_object, "str"_a, "tx_taint_map"_a);
352-
353353
m.def("shift_taint_range",
354354
&api_shift_taint_range,
355355
py::return_value_policy::move,
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
Vulnerability Management for Code-level (IAST): Some native exceptions were not being caught correctly by the python tracer.
5+
This fix remove those exceptions to avoid fatal error executions.

tests/appsec/iast/aspects/test_add_aspect.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
from ddtrace.appsec._iast._taint_tracking import OriginType
66
from ddtrace.appsec._iast._taint_tracking import Source
7+
from ddtrace.appsec._iast._taint_tracking import create_context
8+
from ddtrace.appsec._iast._taint_tracking import destroy_context
79
from ddtrace.appsec._iast._taint_tracking import get_tainted_ranges
810
from ddtrace.appsec._iast._taint_tracking import is_pyobject_tainted
911
from ddtrace.appsec._iast._taint_tracking import taint_pyobject
@@ -301,3 +303,73 @@ def test_taint_ranges_as_evidence_info_different_tainted_op1_and_op3_add():
301303
{"value": tainted_text2, "source": 1},
302304
]
303305
assert sources == [input_info1, input_info2]
306+
307+
308+
def test_taint_object_error_with_no_context():
309+
"""Test taint_pyobject without context. This test is to ensure that the function does not raise an exception."""
310+
string_to_taint = "my_string"
311+
create_context()
312+
result = taint_pyobject(
313+
pyobject=string_to_taint,
314+
source_name="test_add_aspect_tainting_left_hand",
315+
source_value=string_to_taint,
316+
source_origin=OriginType.PARAMETER,
317+
)
318+
319+
ranges_result = get_tainted_ranges(result)
320+
assert len(ranges_result) == 1
321+
322+
destroy_context()
323+
result = taint_pyobject(
324+
pyobject=string_to_taint,
325+
source_name="test_add_aspect_tainting_left_hand",
326+
source_value=string_to_taint,
327+
source_origin=OriginType.PARAMETER,
328+
)
329+
330+
ranges_result = get_tainted_ranges(result)
331+
assert len(ranges_result) == 0
332+
333+
create_context()
334+
result = taint_pyobject(
335+
pyobject=string_to_taint,
336+
source_name="test_add_aspect_tainting_left_hand",
337+
source_value=string_to_taint,
338+
source_origin=OriginType.PARAMETER,
339+
)
340+
341+
ranges_result = get_tainted_ranges(result)
342+
assert len(ranges_result) == 1
343+
344+
345+
def test_get_ranges_from_object_with_no_context():
346+
"""Test taint_pyobject without context. This test is to ensure that the function does not raise an exception."""
347+
string_to_taint = "my_string"
348+
create_context()
349+
result = taint_pyobject(
350+
pyobject=string_to_taint,
351+
source_name="test_add_aspect_tainting_left_hand",
352+
source_value=string_to_taint,
353+
source_origin=OriginType.PARAMETER,
354+
)
355+
356+
destroy_context()
357+
ranges_result = get_tainted_ranges(result)
358+
assert len(ranges_result) == 0
359+
360+
361+
def test_propagate_ranges_with_no_context():
362+
"""Test taint_pyobject without context. This test is to ensure that the function does not raise an exception."""
363+
string_to_taint = "my_string"
364+
create_context()
365+
result = taint_pyobject(
366+
pyobject=string_to_taint,
367+
source_name="test_add_aspect_tainting_left_hand",
368+
source_value=string_to_taint,
369+
source_origin=OriginType.PARAMETER,
370+
)
371+
372+
destroy_context()
373+
result_2 = add_aspect(result, "another_string")
374+
ranges_result = get_tainted_ranges(result_2)
375+
assert len(ranges_result) == 0

0 commit comments

Comments
 (0)