Skip to content

Commit 0e5abc9

Browse files
authored
chore(iast): fix fstring log message (#13601)
Fix IAST validation in fstring aspect. We're considering as unexpected error, format errors like: `Unknown format code 'd' for object of type 'str'` this PR fixes an error in https://github.com/DataDog/dd-trace-py/pull/13600/files#r2128401733 part of #13600 APPSEC-57578 ## 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)
1 parent 0b32ac2 commit 0e5abc9

File tree

9 files changed

+188
-138
lines changed

9 files changed

+188
-138
lines changed

benchmarks/bm/iast_fixtures/str_methods.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,10 +1263,6 @@ def do_customspec_formatspec():
12631263
return f"{c!s:<20s}"
12641264

12651265

1266-
def do_fstring(a, b):
1267-
return f"{a} + {b} = {a + b}"
1268-
1269-
12701266
def _preprocess_lexer_input(text):
12711267
"""Apply preprocessing such as decoding the input, removing BOM and normalizing newlines."""
12721268
# text now *is* a unicode string

benchmarks/bm/iast_fixtures/str_methods_py3.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ def do_fstring(a: str) -> str:
2020
return f"{a}"
2121

2222

23+
def do_fstring_operations(a, b):
24+
return f"{a} + {b} = {a + b}"
25+
26+
2327
def do_zero_padding_fstring(a: int, spec: str = "05d") -> str:
2428
return f"{a:{spec}}"
2529

ddtrace/appsec/_iast/_taint_tracking/aspects.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -449,11 +449,11 @@ def format_value_aspect(
449449
return format(new_text)
450450

451451
if format_spec:
452+
new_new_text = f"{new_text:{format_spec}}" # type: ignore[str-bytes-safe]
452453
try:
453454
# Apply formatting
454455
text_ranges = get_tainted_ranges(new_text)
455456
if text_ranges:
456-
new_new_text = ("{:%s}" % format_spec).format(new_text)
457457
try:
458458
new_ranges = list()
459459
for text_range in text_ranges:
@@ -462,12 +462,12 @@ def format_value_aspect(
462462
taint_pyobject_with_ranges(new_new_text, tuple(new_ranges))
463463
return new_new_text
464464
except ValueError:
465-
return ("{:%s}" % format_spec).format(new_text)
465+
return new_new_text
466466
else:
467-
return ("{:%s}" % format_spec).format(new_text)
467+
return new_new_text
468468
except Exception as e:
469469
iast_propagation_error_log(f"format_value_aspect. {e}")
470-
return ("{:%s}" % format_spec).format(new_text)
470+
return new_new_text
471471

472472
return format(new_text)
473473

tests/appsec/iast/aspects/aspect_utils.py

Lines changed: 41 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -65,56 +65,57 @@ def create_taint_range_with_format(text_input: Any, fn_origin: str = "") -> Any:
6565
return text_output
6666

6767

68-
class BaseReplacement:
69-
def _to_tainted_string_with_origin(self, text: TEXT_TYPE) -> TEXT_TYPE:
70-
if not isinstance(text, (str, bytes, bytearray)):
71-
return text
72-
73-
# CAVEAT: the sequences ":+-" and "-+:" can be escaped with "::++--" and "--+*::"
74-
elements = re.split(r"(\:\+-<[0-9a-zA-Z\-]+>|<[0-9a-zA-Z\-]+>-\+\:)", text)
75-
76-
ranges: List[TaintRange] = []
77-
ranges_append = ranges.append
78-
new_text = text.__class__()
79-
context: Optional[EscapeContext] = None
80-
for index, element in enumerate(elements):
81-
if index % 2 == 0:
82-
element = element.replace("::++--", ":+-")
83-
element = element.replace("--++::", "-+:")
84-
new_text += element
68+
def _to_tainted_string_with_origin(text: TEXT_TYPE) -> TEXT_TYPE:
69+
if not isinstance(text, (str, bytes, bytearray)):
70+
return text
71+
72+
# CAVEAT: the sequences ":+-" and "-+:" can be escaped with "::++--" and "--+*::"
73+
elements = re.split(r"(\:\+-<[0-9a-zA-Z\-]+>|<[0-9a-zA-Z\-]+>-\+\:)", text)
74+
75+
ranges: List[TaintRange] = []
76+
ranges_append = ranges.append
77+
new_text = text.__class__()
78+
context: Optional[EscapeContext] = None
79+
for index, element in enumerate(elements):
80+
if index % 2 == 0:
81+
element = element.replace("::++--", ":+-")
82+
element = element.replace("--++::", "-+:")
83+
new_text += element
84+
else:
85+
separator = element
86+
if element.startswith(":"):
87+
id_evidence = separator[4:-1]
88+
start = len(new_text)
89+
context = EscapeContext(id_evidence, start)
8590
else:
86-
separator = element
87-
if element.startswith(":"):
88-
id_evidence = separator[4:-1]
89-
start = len(new_text)
90-
context = EscapeContext(id_evidence, start)
91-
else:
92-
id_evidence = separator[1:-4]
93-
end = len(new_text)
94-
assert context is not None
95-
start = context.position
96-
if start != end:
97-
assert isinstance(id_evidence, str)
98-
99-
ranges_append(
100-
TaintRange(
101-
start,
102-
end - start,
103-
Source(name=id_evidence, value=new_text[start:], origin=OriginType.PARAMETER),
104-
)
91+
id_evidence = separator[1:-4]
92+
end = len(new_text)
93+
assert context is not None
94+
start = context.position
95+
if start != end:
96+
assert isinstance(id_evidence, str)
97+
98+
ranges_append(
99+
TaintRange(
100+
start,
101+
end - start,
102+
Source(name=id_evidence, value=new_text[start:], origin=OriginType.PARAMETER),
105103
)
106-
set_ranges(new_text, tuple(ranges))
107-
return new_text
104+
)
105+
set_ranges(new_text, tuple(ranges))
106+
return new_text
107+
108108

109+
class BaseReplacement:
109110
def _assert_format_result(
110111
self,
111112
taint_escaped_template: TEXT_TYPE,
112113
taint_escaped_parameter: Any,
113114
expected_result: TEXT_TYPE,
114115
escaped_expected_result: TEXT_TYPE,
115116
) -> None:
116-
template = self._to_tainted_string_with_origin(taint_escaped_template)
117-
parameter = self._to_tainted_string_with_origin(taint_escaped_parameter)
117+
template = _to_tainted_string_with_origin(taint_escaped_template)
118+
parameter = _to_tainted_string_with_origin(taint_escaped_parameter)
118119
result = mod.do_format_with_positional_parameter(template, parameter)
119120

120121
assert result == expected_result

tests/appsec/iast/aspects/test_format_map_aspect_fixtures.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from ddtrace.appsec._iast._taint_tracking import as_formatted_evidence
99
from ddtrace.appsec._iast._taint_tracking import get_ranges
1010
from tests.appsec.iast.aspects.aspect_utils import BaseReplacement
11+
from tests.appsec.iast.aspects.aspect_utils import _to_tainted_string_with_origin
1112
from tests.appsec.iast.iast_utils import _iast_patched_module
1213

1314

@@ -22,8 +23,8 @@ def _assert_format_map_result(
2223
expected_result, # type: str
2324
escaped_expected_result, # type: str
2425
): # type: (...) -> None
25-
template = self._to_tainted_string_with_origin(taint_escaped_template)
26-
mapping = {key: self._to_tainted_string_with_origin(value) for key, value in taint_escaped_mapping.items()}
26+
template = _to_tainted_string_with_origin(taint_escaped_template)
27+
mapping = {key: _to_tainted_string_with_origin(value) for key, value in taint_escaped_mapping.items()}
2728

2829
assert isinstance(template, str)
2930
result = mod.do_format_map(template, mapping)

0 commit comments

Comments
 (0)