Skip to content

Commit 4ae6aca

Browse files
avara1986datadog-datadog-prod-us1[bot]
authored andcommitted
chore(iast): strip, rstrip, lstrip aspects (#13266)
This PR implements new aspects for Python's string strip methods (strip, rstrip, lstrip) to support IAST taint propagation. These aspects ensure proper taint tracking when string stripping operations are performed. ## Motivation - Enable taint propagation through string strip operations - Remove TODO related to strip function and werkzeug - Ensure security marks are properly maintained during string manipulation - Support proper range calculations for tainted strings after stripping ## Changes Made 1. Added new aspects in `ddtrace/appsec/_iast/_taint_tracking/aspects.py`: - `strip_aspect`: Handles both-ends string stripping - `rstrip_aspect`: Handles right-side string stripping - `lstrip_aspect`: Handles left-side string stripping 2. Each aspect implementation: - Maintains taint propagation through strip operations - Recalculates taint ranges based on stripped content - Preserves security marks - Handles both default whitespace stripping and custom character stripping 3. Added comprehensive test coverage in `tests/appsec/iast/aspects/test_strip_aspect.py`: - Property-based tests for basic functionality - Taint tracking verification - Range calculation tests - Edge cases and special character handling - ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
1 parent 6ff3962 commit 4ae6aca

File tree

9 files changed

+534
-17
lines changed

9 files changed

+534
-17
lines changed

benchmarks/appsec_iast_aspects/config.yaml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,30 @@ join_noaspect:
110110
<<: *join_aspect
111111
function_name: "join_noaspect"
112112

113+
strip_aspect: &strip_aspect
114+
warmups: 1
115+
function_name: "iast_strip_aspect"
116+
117+
strip_noaspect:
118+
<<: *strip_aspect
119+
function_name: "strip_noaspect"
120+
121+
rstrip_aspect: &rstrip_aspect
122+
warmups: 1
123+
function_name: "iast_rstrip_aspect"
124+
125+
rstrip_noaspect:
126+
<<: *rstrip_aspect
127+
function_name: "rstrip_noaspect"
128+
129+
lstrip_aspect: &lstrip_aspect
130+
warmups: 1
131+
function_name: "iast_lstrip_aspect"
132+
133+
lstrip_noaspect:
134+
<<: *lstrip_aspect
135+
function_name: "lstrip_noaspect"
136+
113137
lower_aspect: &lower_aspect
114138
warmups: 1
115139
function_name: "iast_lower_aspect"

benchmarks/appsec_iast_aspects/functions.py

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,16 @@
2828
"modulo_aspect",
2929
"replace_aspect",
3030
"repr_aspect",
31-
"rsplit_aspect",
3231
"slice_aspect",
33-
"split_aspect",
34-
"split_aspect",
35-
"splitlines_aspect",
3632
"str_aspect",
3733
"stringio_aspect",
3834
"swapcase_aspect",
3935
"title_aspect",
4036
"translate_aspect",
4137
"upper_aspect",
38+
"rstrip_aspect",
39+
"lstrip_aspect",
40+
"strip_aspect",
4241
]
4342

4443
notfound_symbols = []
@@ -454,3 +453,27 @@ def iast_split_aspect():
454453

455454
def split_noaspect():
456455
return "foo bar baz".split()
456+
457+
458+
def iast_strip_aspect():
459+
return strip_aspect(None, 1, " foo bar baz ") # noqa: F821
460+
461+
462+
def strip_noaspect():
463+
return " foo bar baz ".strip()
464+
465+
466+
def iast_rstrip_aspect():
467+
return rstrip_aspect(None, 1, " foo bar baz ") # noqa: F821
468+
469+
470+
def rstrip_noaspect():
471+
return " foo bar baz ".rstrip()
472+
473+
474+
def iast_lstrip_aspect():
475+
return lstrip_aspect(None, 1, " foo bar baz ") # noqa: F821
476+
477+
478+
def lstrip_noaspect():
479+
return " foo bar baz ".lstrip()

ddtrace/appsec/_iast/_ast/visitor.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ def _mark_avoid_convert_recursively(node):
7171
"split": _PREFIX + "aspects.split_aspect", # Both regular split and re.split
7272
"rsplit": _PREFIX + "aspects.rsplit_aspect",
7373
"splitlines": _PREFIX + "aspects.splitlines_aspect",
74+
"lstrip": _PREFIX + "aspects.lstrip_aspect",
75+
"rstrip": _PREFIX + "aspects.rstrip_aspect",
76+
"strip": _PREFIX + "aspects.strip_aspect",
7477
# re module and re.Match methods
7578
"findall": _PREFIX + "aspects.re_findall_aspect",
7679
"finditer": _PREFIX + "aspects.re_finditer_aspect",

ddtrace/appsec/_iast/_patch_modules.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ def patch_iast(patch_modules=IAST_PATCH):
5757
when_imported("werkzeug.utils")(
5858
lambda _: try_wrap_function_wrapper("werkzeug.utils", "secure_filename", path_traversal_sanitizer)
5959
)
60-
# TODO: werkzeug.utils.safe_join propagation doesn't work because strip("._") which is not yet supported by IAST
60+
# TODO: werkzeug.utils.safe_join propagation doesn't work because normpath which is not yet supported by IAST
6161
# when_imported("werkzeug.utils")(
6262
# lambda _: try_wrap_function_wrapper(
6363
# "werkzeug.utils",

ddtrace/appsec/_iast/_taint_tracking/aspects.py

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@
110110
"slice_aspect",
111111
"split_aspect",
112112
"splitlines_aspect",
113+
"lstrip_aspect",
114+
"rstrip_aspect",
115+
"strip_aspect",
113116
"str_aspect",
114117
"stringio_aspect",
115118
"swapcase_aspect",
@@ -1312,3 +1315,122 @@ def ospathsplitroot_aspect(*args: Any, **kwargs: Any) -> Any:
13121315
iast_propagation_error_log(f"_aspect_ospathsplitroot. {e}")
13131316

13141317
return os.path.splitroot(*args, **kwargs) # type: ignore[attr-defined]
1318+
1319+
1320+
def lstrip_aspect(orig_function: Optional[Callable], flag_added_args: int, *args: Any, **kwargs: Any) -> TEXT_TYPES:
1321+
if orig_function is not None and not isinstance(orig_function, BuiltinFunctionType):
1322+
if flag_added_args > 0:
1323+
args = args[flag_added_args:]
1324+
return orig_function(*args, **kwargs)
1325+
1326+
candidate_text = args[0]
1327+
args = args[flag_added_args:]
1328+
1329+
result = candidate_text.lstrip(*args, **kwargs)
1330+
1331+
if not isinstance(candidate_text, IAST.TEXT_TYPES):
1332+
return result
1333+
1334+
try:
1335+
_strip_lstrip_aspect(candidate_text, result)
1336+
return result
1337+
except Exception as e:
1338+
iast_propagation_error_log(f"lstrip_aspect. {e}")
1339+
1340+
return result
1341+
1342+
1343+
def rstrip_aspect(orig_function: Optional[Callable], flag_added_args: int, *args: Any, **kwargs: Any) -> TEXT_TYPES:
1344+
if orig_function is not None and not isinstance(orig_function, BuiltinFunctionType):
1345+
if flag_added_args > 0:
1346+
args = args[flag_added_args:]
1347+
return orig_function(*args, **kwargs)
1348+
1349+
candidate_text = args[0]
1350+
args = args[flag_added_args:]
1351+
1352+
result = candidate_text.rstrip(*args, **kwargs)
1353+
1354+
if not isinstance(candidate_text, IAST.TEXT_TYPES):
1355+
return result
1356+
1357+
try:
1358+
ranges_new: List[TaintRange] = []
1359+
ranges_new_append = ranges_new.append
1360+
1361+
ranges = get_ranges(candidate_text)
1362+
len_result = len(result)
1363+
if len_result == len(candidate_text):
1364+
taint_pyobject_with_ranges(result, tuple(ranges))
1365+
else:
1366+
for taint_range in ranges:
1367+
if taint_range.start >= len_result:
1368+
continue
1369+
1370+
new_length = min(len_result - taint_range.start, taint_range.length)
1371+
new_range = TaintRange(
1372+
start=taint_range.start,
1373+
length=new_length,
1374+
source=taint_range.source,
1375+
secure_marks=taint_range.secure_marks,
1376+
)
1377+
ranges_new_append(new_range)
1378+
taint_pyobject_with_ranges(result, tuple(ranges_new))
1379+
return result
1380+
except Exception as e:
1381+
iast_propagation_error_log(f"rstrip_aspect. {e}")
1382+
1383+
return result
1384+
1385+
1386+
def strip_aspect(orig_function: Optional[Callable], flag_added_args: int, *args: Any, **kwargs: Any) -> TEXT_TYPES:
1387+
if orig_function is not None and not isinstance(orig_function, BuiltinFunctionType):
1388+
if flag_added_args > 0:
1389+
args = args[flag_added_args:]
1390+
return orig_function(*args, **kwargs)
1391+
1392+
candidate_text = args[0]
1393+
args = args[flag_added_args:]
1394+
result = candidate_text.strip(*args, **kwargs)
1395+
1396+
if not isinstance(candidate_text, IAST.TEXT_TYPES):
1397+
return result
1398+
1399+
try:
1400+
_strip_lstrip_aspect(candidate_text, result)
1401+
return result
1402+
except Exception as e:
1403+
iast_propagation_error_log(f"strip_aspect. {e}")
1404+
1405+
return result
1406+
1407+
1408+
def _strip_lstrip_aspect(candidate_text, result):
1409+
ranges_new: List[TaintRange] = []
1410+
ranges = get_ranges(candidate_text)
1411+
start_pos = candidate_text.index(result)
1412+
len_result = len(result)
1413+
end_pos = start_pos + len_result
1414+
if len_result != len(candidate_text):
1415+
for taint_range in ranges:
1416+
range_start = taint_range.start
1417+
range_end = range_start + taint_range.length
1418+
1419+
if range_end <= start_pos or range_start >= end_pos:
1420+
continue
1421+
1422+
# Calculate new range boundaries
1423+
new_start = max(range_start - start_pos, 0)
1424+
new_end = min(range_end - start_pos, len_result)
1425+
new_length = new_end - new_start
1426+
1427+
if new_length > 0:
1428+
# Create a new range with adjusted position and length
1429+
new_range = TaintRange(
1430+
start=new_start,
1431+
length=new_length,
1432+
source=taint_range.source,
1433+
secure_marks=taint_range.secure_marks,
1434+
)
1435+
ranges_new.append(new_range)
1436+
taint_pyobject_with_ranges(result, tuple(ranges_new))

scripts/iast/mod_leak_functions.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,14 +308,16 @@ async def test_doit():
308308
string12 = string11 + "_notainted"
309309
string13 = string12.rsplit("_", 1)[0]
310310
string13_2 = string13 + " " + string13
311+
string13_3 = string13_2.strip()
312+
string13_4 = string13_3.rstrip()
313+
string13_5 = string13_4.lstrip()
311314
try:
312-
string13_3, string13_5, string13_5 = string13_2.split(" ")
315+
string13_5_1, string13_5_2, string13_5_3 = string13_5.split(" ")
313316
except ValueError:
314317
pass
315-
sink_points(string13_2)
316-
318+
sink_points(string13_5)
317319
# os path propagation
318-
string14 = os.path.join(string13_2, "a")
320+
string14 = os.path.join(string13_5, "a")
319321
string15 = os.path.split(string14)[0]
320322
string16 = os.path.dirname(string15 + "/" + "foobar")
321323
string17 = os.path.basename("/foobar/" + string16)

0 commit comments

Comments
 (0)