Skip to content

Commit

Permalink
Merge branch '2.10' into backport-10019-to-2.10
Browse files Browse the repository at this point in the history
  • Loading branch information
mabdinur authored Aug 12, 2024
2 parents 3977bad + 8b587b9 commit 93da63c
Show file tree
Hide file tree
Showing 13 changed files with 268 additions and 161 deletions.
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ mypy.ini @DataDog/python-guild @DataDog/apm-core-pyt
ddtrace/internal/_file_queue.py @DataDog/python-guild
ddtrace/internal/_unpatched.py @DataDog/python-guild
ddtrace/internal/compat.py @DataDog/python-guild @DataDog/apm-core-python
ddtrace/settings/config.py @DataDog/python-guild @DataDog/apm-sdk-api-python
docs/ @DataDog/python-guild
tests/utils.py @DataDog/python-guild
tests/.suitespec.json @DataDog/python-guild @DataDog/apm-core-python
tests/suitespec.py @DataDog/python-guild @DataDog/apm-core-python
Expand Down
1 change: 1 addition & 0 deletions .gitlab/build-oci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ cp ../lib-injection/sitecustomize.py $BUILD_DIR/
cp auto_inject-python.version $BUILD_DIR/version
cp ../min_compatible_versions.csv $BUILD_DIR/
cp ../lib-injection/telemetry-forwarder.sh $BUILD_DIR/
chmod -R +r $BUILD_DIR
chmod -R o-w $BUILD_DIR
chmod -R g-w $BUILD_DIR

Expand Down
48 changes: 41 additions & 7 deletions ddtrace/appsec/_iast/_taint_tracking/Utils/StringUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,31 +78,65 @@ PyObjectToString(PyObject* obj)
PyObject*
new_pyobject_id(PyObject* tainted_object)
{
if (!tainted_object)
return nullptr;

if (PyUnicode_Check(tainted_object)) {
PyObject* empty_unicode = PyUnicode_New(0, 127);
if (!empty_unicode)
return tainted_object;
PyObject* val = Py_BuildValue("(OO)", tainted_object, empty_unicode);
if (!val) {
Py_XDECREF(empty_unicode);
return tainted_object;
}
PyObject* result = PyUnicode_Join(empty_unicode, val);
Py_DecRef(empty_unicode);
Py_DecRef(val);
if (!result) {
result = tainted_object;
}
Py_XDECREF(empty_unicode);
Py_XDECREF(val);
return result;
}
if (PyBytes_Check(tainted_object)) {
PyObject* empty_bytes = PyBytes_FromString("");
if (!empty_bytes)
return tainted_object;

const auto bytes_join_ptr = py::reinterpret_borrow<py::bytes>(empty_bytes).attr("join");
const auto val = Py_BuildValue("(OO)", tainted_object, empty_bytes);
if (!val or !bytes_join_ptr.ptr()) {
Py_XDECREF(empty_bytes);
return tainted_object;
}

const auto res = PyObject_CallFunctionObjArgs(bytes_join_ptr.ptr(), val, NULL);
Py_DecRef(val);
Py_DecRef(empty_bytes);
Py_XDECREF(val);
Py_XDECREF(empty_bytes);
return res;
} else if (PyByteArray_Check(tainted_object)) {
PyObject* empty_bytes = PyBytes_FromString("");
if (!empty_bytes)
return tainted_object;

PyObject* empty_bytearray = PyByteArray_FromObject(empty_bytes);
if (!empty_bytearray) {
Py_XDECREF(empty_bytes);
return tainted_object;
}

const auto bytearray_join_ptr = py::reinterpret_borrow<py::bytes>(empty_bytearray).attr("join");
const auto val = Py_BuildValue("(OO)", tainted_object, empty_bytearray);
if (!val or !bytearray_join_ptr.ptr()) {
Py_XDECREF(empty_bytes);
Py_XDECREF(empty_bytearray);
return tainted_object;
}

const auto res = PyObject_CallFunctionObjArgs(bytearray_join_ptr.ptr(), val, NULL);
Py_DecRef(val);
Py_DecRef(empty_bytes);
Py_DecRef(empty_bytearray);
Py_XDECREF(val);
Py_XDECREF(empty_bytes);
Py_XDECREF(empty_bytearray);
return res;
}
return tainted_object;
Expand Down
3 changes: 2 additions & 1 deletion ddtrace/contrib/tornado/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ def execute(func, handler, args, kwargs):
request_span.set_tag(ANALYTICS_SAMPLE_RATE_KEY, settings.get("analytics_sample_rate", True))

http_route = _find_route(handler.application.default_router.rules, handler.request)
request_span.set_tag_str("http.route", http_route)
if http_route is not None and isinstance(http_route, str):
request_span.set_tag_str("http.route", http_route)
setattr(handler.request, REQUEST_SPAN_KEY, request_span)

return func(*args, **kwargs)
Expand Down
40 changes: 20 additions & 20 deletions ddtrace/profiling/collector/_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,13 @@ def _acquire(self, inner_func, *args, **kwargs):
end = self._self_acquired_at = compat.monotonic_ns()
thread_id, thread_name = _current_thread()
task_id, task_name, task_frame = _task.get_task(thread_id)
lock_name = self._get_lock_call_loc_with_name() or self._self_init_loc
self._maybe_update_self_name()
lock_name = "%s:%s" % (self._self_init_loc, self._self_name) if self._self_name else self._self_init_loc

if task_frame is None:
frame = sys._getframe(1)
# If we can't get the task frame, we use the caller frame. We expect acquire/release or
# __enter__/__exit__ to be on the stack, so we go back 2 frames.
frame = sys._getframe(2)
else:
frame = task_frame

Expand Down Expand Up @@ -172,10 +175,13 @@ def _release(self, inner_func, *args, **kwargs):
end = compat.monotonic_ns()
thread_id, thread_name = _current_thread()
task_id, task_name, task_frame = _task.get_task(thread_id)
lock_name = self._get_lock_call_loc_with_name() or self._self_init_loc
lock_name = (
"%s:%s" % (self._self_init_loc, self._self_name) if self._self_name else self._self_init_loc
)

if task_frame is None:
frame = sys._getframe(1)
# See the comments in _acquire
frame = sys._getframe(2)
else:
frame = task_frame

Expand Down Expand Up @@ -237,23 +243,23 @@ def __enter__(self, *args, **kwargs):
def __exit__(self, *args, **kwargs):
self._release(self.__wrapped__.__exit__, *args, **kwargs)

def _maybe_update_lock_name(self, var_dict: typing.Dict):
if self._self_name:
return
def _find_self_name(self, var_dict: typing.Dict):
for name, value in var_dict.items():
if name.startswith("__") or isinstance(value, types.ModuleType):
continue
if value is self:
self._self_name = name
break
return name
if config.lock.name_inspect_dir:
for attribute in dir(value):
if not attribute.startswith("__") and getattr(value, attribute) is self:
self._self_name = attribute
break
return attribute
return None

# Get lock acquire/release call location and variable name the lock is assigned to
def _get_lock_call_loc_with_name(self) -> typing.Optional[str]:
def _maybe_update_self_name(self):
if self._self_name:
return
try:
# We expect the call stack to be like this:
# 0: this
Expand All @@ -268,24 +274,18 @@ def _get_lock_call_loc_with_name(self) -> typing.Optional[str]:
if frame.f_code.co_name not in {"acquire", "release", "__enter__", "__exit__"}:
raise AssertionError("Unexpected frame %s" % frame.f_code.co_name)
frame = sys._getframe(3)
code = frame.f_code
call_loc = "%s:%d" % (os.path.basename(code.co_filename), frame.f_lineno)

# First, look at the local variables of the caller frame, and then the global variables
self._maybe_update_lock_name(frame.f_locals)
self._maybe_update_lock_name(frame.f_globals)
self._self_name = self._find_self_name(frame.f_locals) or self._find_self_name(frame.f_globals)

if self._self_name:
return "%s:%s" % (call_loc, self._self_name)
else:
if not self._self_name:
self._self_name = ""
LOG.warning(
"Failed to get lock variable name, we only support local/global variables and their attributes."
)
return call_loc

except Exception as e:
LOG.warning("Error getting lock acquire/release call location and variable name: %s", e)
return None


class FunctionWrapper(wrapt.FunctionWrapper):
Expand Down
5 changes: 5 additions & 0 deletions releasenotes/notes/fix-ssi-permissions-647693af3d5ce49d.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
fixes:
- |
SSI: This fixes incorrect file permissions on lib-injection images for 2.10.x releases.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
fixes:
- |
Code Security: add null pointer checks when creating new objects ids.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
fixes:
- |
profiling: show lock init location in Lock Name and hide profiler internal
frames from Stack Frame in Timeline Details tab.
4 changes: 3 additions & 1 deletion tests/appsec/iast_memcheck/test_iast_mem_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ def test_propagation_memory_check(origin1, origin2, iast_span_defaults):
_initializer_size = initializer_size()
assert _initializer_size > 0

assert _num_objects_tainted == num_objects_tainted()
# Some tainted pyobject is freed, and Python may reuse the memory address
# hence the number of tainted objects may be the same or less
assert _num_objects_tainted in (num_objects_tainted(), num_objects_tainted() - 1)
assert _active_map_addreses_size == active_map_addreses_size()
assert _initializer_size == initializer_size()
reset_context()
Expand Down
40 changes: 22 additions & 18 deletions tests/profiling/collector/test_asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ async def test_lock_acquire_events():
assert len(r.events[collector_asyncio.AsyncioLockAcquireEvent]) == 1
assert len(r.events[collector_asyncio.AsyncioLockReleaseEvent]) == 0
event = r.events[collector_asyncio.AsyncioLockAcquireEvent][0]
assert event.lock_name == "test_asyncio.py:16:lock"
assert event.lock_name == "test_asyncio.py:15:lock"
assert event.thread_id == _thread.get_ident()
assert event.wait_time_ns >= 0
# It's called through pytest so I'm sure it's gonna be that long, right?
Expand All @@ -39,7 +39,7 @@ async def test_asyncio_lock_release_events():
assert len(r.events[collector_asyncio.AsyncioLockAcquireEvent]) == 1
assert len(r.events[collector_asyncio.AsyncioLockReleaseEvent]) == 1
event = r.events[collector_asyncio.AsyncioLockReleaseEvent][0]
assert event.lock_name == "test_asyncio.py:38:lock"
assert event.lock_name == "test_asyncio.py:35:lock"
assert event.thread_id == _thread.get_ident()
assert event.locked_for_ns >= 0
# It's called through pytest so I'm sure it's gonna be that long, right?
Expand Down Expand Up @@ -69,23 +69,27 @@ async def test_lock_events_tracer(tracer):
pass
events = r.reset()
# The tracer might use locks, so we need to look into every event to assert we got ours
lock1_acquire, lock1_release, lock2_acquire, lock2_release = (
"test_asyncio.py:59:lock",
"test_asyncio.py:63:lock",
"test_asyncio.py:62:lock2",
"test_asyncio.py:65:lock2",
)
lock1_name = "test_asyncio.py:58:lock"
lock2_name = "test_asyncio.py:61:lock2"
lines_with_trace = [61, 63]
lines_without_trace = [59, 65]
for event_type in (collector_asyncio.AsyncioLockAcquireEvent, collector_asyncio.AsyncioLockReleaseEvent):
if event_type == collector_asyncio.AsyncioLockAcquireEvent:
assert {lock1_acquire, lock2_acquire}.issubset({e.lock_name for e in events[event_type]})
assert {lock1_name, lock2_name}.issubset({e.lock_name for e in events[event_type]})
elif event_type == collector_asyncio.AsyncioLockReleaseEvent:
assert {lock1_release, lock2_release}.issubset({e.lock_name for e in events[event_type]})
assert {lock1_name, lock2_name}.issubset({e.lock_name for e in events[event_type]})
for event in events[event_type]:
if event.lock_name in [lock1_acquire, lock2_release]:
assert event.span_id is None
assert event.trace_resource_container is None
assert event.trace_type is None
elif event.lock_name in [lock2_acquire, lock1_release]:
assert event.span_id == span_id
assert event.trace_resource_container[0] == t.resource
assert event.trace_type == t.span_type
if event.name in [lock1_name, lock2_name]:
file_name, lineno, function_name, class_name = event.frames[0]
assert file_name == __file__.replace(".pyc", ".py")
assert lineno in lines_with_trace + lines_without_trace
assert function_name == "test_lock_events_tracer"
assert class_name == ""
if lineno in lines_without_trace:
assert event.span_id is None
assert event.trace_resource_container is None
assert event.trace_type is None
elif lineno in lines_with_trace:
assert event.span_id == span_id
assert event.trace_resource_container[0] == resource
assert event.trace_type == span_type
Loading

0 comments on commit 93da63c

Please sign in to comment.