Skip to content

Commit 71055e9

Browse files
authored
fix(iast): fix memory leak error [backport #7788 to 2.3] (#7890)
This below backports list to 2.3. - chore(iast): `format_aspect` refactor #7884 and #7772 - fix(iast): iast leak for python 3.11: #7788 - chore(iast): add propagation for str aspect join: #6940 - chore(iast): add collision tests for bytes and bytearray: #7367 - chore(iast): store hash and id separately in tx_taint_map: #7340 ## 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] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [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)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly 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) - [x] If this PR 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`. - [x] This PR doesn't touch any of that.
1 parent 3b5bf70 commit 71055e9

39 files changed

+539
-446
lines changed

ddtrace/appsec/_iast/_ast/ast_patching.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
import os
66
import re
77
from sys import builtin_module_names
8-
from typing import TYPE_CHECKING
8+
from types import ModuleType
9+
from typing import TYPE_CHECKING # noqa:F401
10+
from typing import Tuple
911

1012

1113
if TYPE_CHECKING:
12-
from types import ModuleType
13-
from typing import Optional
14-
from typing import Tuple
14+
from typing import Optional # noqa:F401
1515

1616
from ddtrace.appsec._constants import IAST
1717
from ddtrace.appsec._python_info.stdlib import _stdlib_for_python_version
@@ -123,8 +123,7 @@ def _remove_flask_run(text): # type (str) -> str
123123
return new_text
124124

125125

126-
def astpatch_module(module, remove_flask_run=False):
127-
# type: (ModuleType, bool) -> Tuple[str, str]
126+
def astpatch_module(module: ModuleType, remove_flask_run: bool = False) -> Tuple[str, str]:
128127
module_name = module.__name__
129128
module_path = str(origin(module))
130129
try:

ddtrace/appsec/_iast/_ast/visitor.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
import ast
55
import copy
66
import sys
7-
from typing import Any
8-
from typing import List
9-
from typing import Set
7+
from typing import Any # noqa:F401
8+
from typing import List # noqa:F401
9+
from typing import Set # noqa:F401
1010

1111
from six import iteritems
1212

ddtrace/appsec/_iast/_overhead_control_engine.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,18 @@
55
"""
66
import os
77
import threading
8-
from typing import TYPE_CHECKING
8+
from typing import TYPE_CHECKING # noqa:F401
99

1010
from ddtrace.internal.logger import get_logger
1111
from ddtrace.sampler import RateSampler
1212

1313

1414
if TYPE_CHECKING: # pragma: no cover
15-
from typing import Set
16-
from typing import Tuple
17-
from typing import Type
15+
from typing import Set # noqa:F401
16+
from typing import Tuple # noqa:F401
17+
from typing import Type # noqa:F401
1818

19-
from ddtrace.span import Span
19+
from ddtrace.span import Span # noqa:F401
2020

2121
log = get_logger(__name__)
2222

@@ -98,7 +98,8 @@ class OverheadControl(object):
9898
def reconfigure(self):
9999
self._sampler = RateSampler(sample_rate=get_request_sampling_value() / 100.0)
100100

101-
def acquire_request(self, span): # type: (Span) -> None
101+
def acquire_request(self, span):
102+
# type: (Span) -> None
102103
"""Decide whether if IAST analysis will be done for this request.
103104
- Block a request's quota at start of the request to limit simultaneous requests analyzed.
104105
- Use sample rating to analyze only a percentage of the total requests (30% by default).

ddtrace/appsec/_iast/_patch.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import ctypes
22
import gc
33
import sys
4-
from typing import TYPE_CHECKING
4+
from typing import TYPE_CHECKING # noqa:F401
55

66
from ddtrace.internal.logger import get_logger
77
from ddtrace.vendor.wrapt import FunctionWrapper
@@ -11,10 +11,10 @@
1111

1212

1313
if TYPE_CHECKING: # pragma: no cover
14-
from typing import Any
15-
from typing import Callable
16-
from typing import Dict
17-
from typing import Optional
14+
from typing import Any # noqa:F401
15+
from typing import Callable # noqa:F401
16+
from typing import Dict # noqa:F401
17+
from typing import Optional # noqa:F401
1818

1919

2020
_DD_ORIGINAL_ATTRIBUTES = {} # type: Dict[Any, Any]

ddtrace/appsec/_iast/_stacktrace.c

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,22 @@
1717
#define GET_LINENO(frame) PyFrame_GetLineNumber((PyFrameObject*)frame)
1818
#define GET_FRAME(tstate) PyThreadState_GetFrame(tstate)
1919
#define GET_PREVIOUS(frame) PyFrame_GetBack(frame)
20-
#define FRAME_DECREF(frame) Py_DECREF(frame)
20+
#define FRAME_DECREF(frame) Py_DecRef(frame)
2121
#define FRAME_XDECREF(frame) Py_XDECREF(frame)
22-
#define FILENAME_DECREF(filename) Py_DECREF(filename)
23-
#define FILENAME_XDECREF(filename) Py_XDECREF(filename)
22+
#define FILENAME_DECREF(filename) Py_DecRef(filename)
23+
#define FILENAME_XDECREF(filename) \
24+
if (filename) \
25+
Py_DecRef(filename)
2426
static inline PyObject*
2527
GET_FILENAME(PyFrameObject* frame)
2628
{
2729
PyCodeObject* code = PyFrame_GetCode(frame);
2830
if (!code) {
29-
Py_DECREF(code);
3031
return NULL;
3132
}
32-
return PyObject_GetAttrString((PyObject*)code, "co_filename");
33+
PyObject* filename = PyObject_GetAttrString((PyObject*)code, "co_filename");
34+
Py_DecRef(code);
35+
return filename;
3336
}
3437
#else
3538
#define GET_FRAME(tstate) tstate->frame
@@ -113,7 +116,7 @@ get_file_and_line(PyObject* Py_UNUSED(module), PyObject* cwd_obj)
113116
}
114117

115118
exit:
116-
Py_DECREF(cwd_bytes);
119+
Py_DecRef(cwd_bytes);
117120
FRAME_XDECREF(frame);
118121
FILENAME_XDECREF(filename_o);
119122
return result;
@@ -123,10 +126,10 @@ exit_0:; // fix: "a label can only be part of a statement and a declaration is n
123126
PyObject* line_obj = Py_BuildValue("i", 0);
124127
filename_o = PyUnicode_FromString("");
125128
result = PyTuple_Pack(2, filename_o, line_obj);
126-
Py_DECREF(cwd_bytes);
129+
Py_DecRef(cwd_bytes);
127130
FRAME_XDECREF(frame);
128131
FILENAME_XDECREF(filename_o);
129-
Py_DECREF(line_obj);
132+
Py_DecRef(line_obj);
130133
return result;
131134
}
132135

ddtrace/appsec/_iast/_taint_dict.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
#!/usr/bin/env python3
22
#
3-
from typing import TYPE_CHECKING
3+
from typing import TYPE_CHECKING # noqa:F401
44

55

66
if TYPE_CHECKING:
7-
from typing import Dict
8-
from typing import Tuple
7+
from typing import Dict # noqa:F401
8+
from typing import Tuple # noqa:F401
99

10-
from ._taint_tracking import Source
10+
from ._taint_tracking import Source # noqa:F401
1111

1212
_IAST_TAINT_DICT = {} # type: Dict[int, Tuple[Tuple[Source, int, int],...]]
1313

ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectExtend.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#include "AspectExtend.h"
2-
#include "Initializer/Initializer.h"
32

43
PyObject*
54
api_extend_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
@@ -28,7 +27,7 @@ api_extend_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs)
2827
// Ensure no returns are done before this method call
2928
auto method_name = PyUnicode_FromString("extend");
3029
PyObject_CallMethodObjArgs(candidate_text, method_name, to_add, nullptr);
31-
Py_DECREF(method_name);
30+
Py_DecRef(method_name);
3231

3332
if (not ctx_map or ctx_map->empty() or to_result == nullptr) {
3433
Py_RETURN_NONE;

ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectExtend.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include "Initializer/Initializer.h"
34
#include <Python.h>
45

56
PyObject*
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
#pragma once
2+
#include "Aspects/AspectFormat.h"
3+
4+
template<class StrType>
5+
StrType
6+
api_format_aspect(StrType& candidate_text,
7+
const py::tuple& parameter_list,
8+
const py::args& args,
9+
const py::kwargs& kwargs)
10+
{
11+
auto [ranges_orig, candidate_text_ranges] = are_all_text_all_ranges(candidate_text.ptr(), parameter_list);
12+
13+
if (!ranges_orig.empty() or !candidate_text_ranges.empty()) {
14+
auto new_template =
15+
_int_as_formatted_evidence<StrType>(candidate_text, candidate_text_ranges, TagMappingMode::Mapper);
16+
17+
py::list new_args;
18+
py::dict new_kwargs;
19+
for (const auto arg : args) {
20+
if (is_text(arg.ptr())) {
21+
auto str_arg = py::cast<py::str>(arg);
22+
auto n_arg = _all_as_formatted_evidence<py::str>(str_arg, TagMappingMode::Mapper);
23+
new_args.append(n_arg);
24+
} else {
25+
new_args.append(arg);
26+
}
27+
}
28+
for (auto [key, value] : kwargs) {
29+
if (is_text(value.ptr())) {
30+
auto str_value = py::cast<py::str>(value);
31+
auto n_value = _all_as_formatted_evidence<py::str>(str_value, TagMappingMode::Mapper);
32+
new_kwargs[key] = n_value;
33+
} else {
34+
new_kwargs[key] = value;
35+
}
36+
}
37+
StrType new_template_format =
38+
py::getattr(new_template, "format")(*(py::cast<py::tuple>(new_args)), **new_kwargs);
39+
std::tuple result = _convert_escaped_text_to_taint_text<StrType>(new_template_format, ranges_orig);
40+
StrType result_text = get<0>(result);
41+
TaintRangeRefs result_ranges = get<1>(result);
42+
PyObject* new_result = new_pyobject_id(result_text.ptr());
43+
set_ranges(new_result, result_ranges);
44+
return py::reinterpret_steal<StrType>(new_result);
45+
}
46+
return py::getattr(candidate_text, "format")(*args, **kwargs);
47+
}
48+
49+
void
50+
pyexport_format_aspect(py::module& m)
51+
{
52+
m.def("_format_aspect", &api_format_aspect<py::str>, "candidate_text"_a, "parameter_list"_a);
53+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#pragma once
2+
#include "Aspects/Helpers.h"
3+
#include "Initializer/Initializer.h"
4+
#include "TaintTracking/Source.h"
5+
#include "TaintTracking/TaintRange.h"
6+
#include "TaintTracking/TaintedObject.h"
7+
8+
template<class StrType>
9+
StrType
10+
api_format_aspect(StrType& candidate_text,
11+
const py::tuple& parameter_list,
12+
const py::args& args,
13+
const py::kwargs& kwargs);
14+
15+
void
16+
pyexport_format_aspect(py::module& m);

0 commit comments

Comments
 (0)