Skip to content

Commit

Permalink
chore(iast): remove new_pyobject_id param (#7103)
Browse files Browse the repository at this point in the history
IAST C++ refactor, remove `object_len` from `new_pyobject_id` C++
function

## 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.
  • Loading branch information
avara1986 authored Sep 29, 2023
1 parent 2a220ba commit c26e86c
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 131 deletions.
3 changes: 1 addition & 2 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
PyObject*
index_aspect(PyObject* result_o, PyObject* candidate_text, PyObject* idx, TaintRangeMapType* tx_taint_map)
{
size_t len_result_o{ get_pyobject_size(result_o) };
auto idx_long = PyLong_AsLong(idx);
TaintRangeRefs ranges_to_set;
auto ranges = get_ranges(candidate_text);
Expand All @@ -16,7 +15,7 @@ index_aspect(PyObject* result_o, PyObject* candidate_text, PyObject* idx, TaintR
}
}

const auto& res_new_id = new_pyobject_id(result_o, len_result_o);
const auto& res_new_id = new_pyobject_id(result_o);
Py_DECREF(result_o);

if (ranges_to_set.empty()) {
Expand Down
4 changes: 2 additions & 2 deletions ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectJoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ aspect_join_str(PyObject* sep,
}
}

PyObject* new_result{ new_pyobject_id(result, PyUnicode_GET_LENGTH(result)) };
PyObject* new_result{ new_pyobject_id(result) };
set_tainted_object(new_result, result_to, tx_taint_map);
Py_DECREF(result);
return new_result;
Expand Down Expand Up @@ -132,7 +132,7 @@ aspect_join(PyObject* sep, PyObject* result, PyObject* iterable_elements, TaintR
}
}

PyObject* new_result{ new_pyobject_id(result, get_pyobject_size(result)) };
PyObject* new_result{ new_pyobject_id(result) };
set_tainted_object(new_result, result_to, tx_taint_map);
Py_DECREF(result);
return new_result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ add_aspect(PyObject* result_o, PyObject* candidate_text, PyObject* text_to_add,
{
size_t len_candidate_text{ get_pyobject_size(candidate_text) };
size_t len_text_to_add{ get_pyobject_size(text_to_add) };
size_t len_result_o{ get_pyobject_size(result_o) };

if (len_text_to_add == 0 and len_candidate_text > 0) {
return candidate_text;
Expand All @@ -26,7 +25,7 @@ add_aspect(PyObject* result_o, PyObject* candidate_text, PyObject* text_to_add,

const auto& to_candidate_text = get_tainted_object(candidate_text, tx_taint_map);
if (to_candidate_text and to_candidate_text->get_ranges().size() >= TaintedObject::TAINT_RANGE_LIMIT) {
const auto& res_new_id = new_pyobject_id(result_o, len_result_o);
const auto& res_new_id = new_pyobject_id(result_o);
Py_DECREF(result_o);
// If left side is already at the maximum taint ranges, we just reuse its
// ranges, we don't need to look at left side.
Expand All @@ -39,15 +38,15 @@ add_aspect(PyObject* result_o, PyObject* candidate_text, PyObject* text_to_add,
return result_o;
}
if (!to_text_to_add) {
const auto& res_new_id = new_pyobject_id(result_o, len_result_o);
const auto& res_new_id = new_pyobject_id(result_o);
Py_DECREF(result_o);
set_tainted_object(res_new_id, to_candidate_text, tx_taint_map);
return res_new_id;
}

auto tainted = initializer->allocate_tainted_object_copy(to_candidate_text);
tainted->add_ranges_shifted(to_text_to_add, (long)len_candidate_text);
const auto& res_new_id = new_pyobject_id(result_o, len_result_o);
const auto& res_new_id = new_pyobject_id(result_o);
Py_DECREF(result_o);
set_tainted_object(res_new_id, tainted, tx_taint_map);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "TaintedOps.h"

PyObject*
new_pyobject_id(PyObject* tainted_object, Py_ssize_t object_length)
new_pyobject_id(PyObject* tainted_object)
{
if (PyUnicode_Check(tainted_object)) {
PyObject* empty_unicode = PyUnicode_New(0, 127);
Expand Down Expand Up @@ -37,9 +37,8 @@ PyObject*
api_new_pyobject_id(PyObject* Py_UNUSED(module), PyObject* args)
{
PyObject* tainted_object;
Py_ssize_t object_length;
PyArg_ParseTuple(args, "On", &tainted_object, &object_length);
return new_pyobject_id(tainted_object, object_length);
PyArg_ParseTuple(args, "O", &tainted_object);
return new_pyobject_id(tainted_object);
}

bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ using namespace pybind11::literals;
namespace py = pybind11;

PyObject*
new_pyobject_id(PyObject* tainted_object, Py_ssize_t object_length);
new_pyobject_id(PyObject* tainted_object);

PyObject*
api_new_pyobject_id(PyObject* Py_UNUSED(module), PyObject* args);
Expand Down
10 changes: 4 additions & 6 deletions ddtrace/appsec/_iast/_taint_tracking/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,26 +78,24 @@
]


def taint_pyobject(pyobject, source_name, source_value, source_origin=None, start=0, len_pyobject=None):
# type: (Any, Any, Any, OriginType, int, Optional[int]) -> Any
def taint_pyobject(pyobject, source_name, source_value, source_origin=None):
# type: (Any, Any, Any, OriginType) -> Any
# Request is not analyzed
if not oce.request_has_quota:
return pyobject
# Pyobject must be Text with len > 1
if not pyobject or not isinstance(pyobject, (str, bytes, bytearray)):
return pyobject

if not len_pyobject:
len_pyobject = len(pyobject)
pyobject_newid = new_pyobject_id(pyobject, len_pyobject)
pyobject_newid = new_pyobject_id(pyobject)
if isinstance(source_name, (bytes, bytearray)):
source_name = str(source_name, encoding="utf8")
if isinstance(source_value, (bytes, bytearray)):
source_value = str(source_value, encoding="utf8")
if source_origin is None:
source_origin = OriginType.PARAMETER
source = Source(source_name, source_value, source_origin)
pyobject_range = TaintRange(start, len_pyobject, source)
pyobject_range = TaintRange(0, len(pyobject), source)
set_ranges(pyobject_newid, [pyobject_range])
_set_metric_iast_executed_source(source_origin)
return pyobject_newid
Expand Down
Loading

0 comments on commit c26e86c

Please sign in to comment.