Skip to content

Commit 466106a

Browse files
avara1986gnufede
andauthored
fix(iast): decouple executed sink metric call [backport 2.0] (#7177)
Backport 0a82754 from #7149 to 2.0. IAST: Fix executed sink telemetry metric by extracting it from the report and calling it separately, as we may reach the sink point without reporting a vulnerability. Testing strategy: Existing telemetry system tests (there is a corner case that they are not covering at the moment) ## 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. Co-authored-by: Federico Mon <[email protected]>
1 parent 34462ac commit 466106a

File tree

10 files changed

+26
-3
lines changed

10 files changed

+26
-3
lines changed

ddtrace/appsec/_iast/taint_sinks/_base.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from ddtrace.settings import _config
1212

1313
from .. import oce
14-
from .._metrics import _set_metric_iast_executed_sink
1514
from .._overhead_control_engine import Operation
1615
from .._utils import _has_to_scrub
1716
from .._utils import _is_evidence_value_parts
@@ -132,8 +131,6 @@ def report(cls, evidence_value="", sources=None):
132131
log.debug("Unexpected evidence_value type: %s", type(evidence_value))
133132
evidence = Evidence(value="")
134133

135-
_set_metric_iast_executed_sink(cls.vulnerability_type)
136-
137134
report = core.get_item(IAST.CONTEXT_KEY, span=span)
138135
if report:
139136
report.vulnerabilities.add(

ddtrace/appsec/_iast/taint_sinks/ast_taint.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from typing import TYPE_CHECKING
22

3+
from .._metrics import _set_metric_iast_executed_sink
34
from ..constants import DEFAULT_WEAK_RANDOMNESS_FUNCTIONS
45
from .weak_randomness import WeakRandomness
56

@@ -26,6 +27,7 @@ def ast_funcion(
2627

2728
if cls.__class__.__module__ == "random" and cls_name == "Random" and func_name in DEFAULT_WEAK_RANDOMNESS_FUNCTIONS:
2829
# Weak, run the analyzer
30+
_set_metric_iast_executed_sink(WeakRandomness.vulnerability_type)
2931
WeakRandomness.report(evidence_value=cls_name + "." + func_name)
3032

3133
return func(*args, **kwargs)

ddtrace/appsec/_iast/taint_sinks/command_injection.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ def _redact_report(cls, report): # type: (IastSpanReporter) -> IastSpanReporter
172172
def _iast_report_cmdi(shell_args):
173173
# type: (Union[str, List[str]]) -> None
174174
report_cmdi = ""
175+
from .._metrics import _set_metric_iast_executed_sink
175176
from .._taint_tracking import get_tainted_ranges
176177
from .._taint_tracking.aspects import join_aspect
177178

@@ -184,4 +185,5 @@ def _iast_report_cmdi(shell_args):
184185
report_cmdi = shell_args
185186

186187
if report_cmdi:
188+
_set_metric_iast_executed_sink(CommandInjection.vulnerability_type)
187189
CommandInjection.report(evidence_value=report_cmdi)

ddtrace/appsec/_iast/taint_sinks/insecure_cookie.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from ddtrace.internal.compat import six
44

55
from .. import oce
6+
from .._metrics import _set_metric_iast_executed_sink
67
from ..constants import EVIDENCE_COOKIE
78
from ..constants import VULN_INSECURE_COOKIE
89
from ..constants import VULN_NO_HTTPONLY_COOKIE
@@ -46,10 +47,12 @@ def asm_check_cookies(cookies): # type: (Optional[Dict[str, str]]) -> None
4647
evidence = "%s=%s" % (cookie_key, cookie_value)
4748

4849
if ";secure" not in lvalue:
50+
_set_metric_iast_executed_sink(InsecureCookie.vulnerability_type)
4951
InsecureCookie.report(evidence_value=evidence)
5052
return
5153

5254
if ";httponly" not in lvalue:
55+
_set_metric_iast_executed_sink(NoHttpOnlyCookie.vulnerability_type)
5356
NoHttpOnlyCookie.report(evidence_value=evidence)
5457
return
5558

@@ -66,3 +69,4 @@ def asm_check_cookies(cookies): # type: (Optional[Dict[str, str]]) -> None
6669

6770
if report_samesite:
6871
NoSameSite.report(evidence_value=evidence)
72+
_set_metric_iast_executed_sink(NoSameSite.vulnerability_type)

ddtrace/appsec/_iast/taint_sinks/path_traversal.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,10 @@ def patch():
4747
def open_path_traversal(*args, **kwargs):
4848
if oce.request_has_quota and PathTraversal.has_quota():
4949
try:
50+
from .._metrics import _set_metric_iast_executed_sink
5051
from .._taint_tracking import is_pyobject_tainted
5152

53+
_set_metric_iast_executed_sink(PathTraversal.vulnerability_type)
5254
if is_pyobject_tainted(args[0]):
5355
PathTraversal.report(evidence_value=args[0])
5456
except Exception:

ddtrace/appsec/_iast/taint_sinks/weak_cipher.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from ddtrace.internal.logger import get_logger
55

66
from .. import oce
7+
from .._metrics import _set_metric_iast_executed_sink
78
from .._metrics import _set_metric_iast_instrumented_sink
89
from .._patch import set_and_check_module_is_patched
910
from .._patch import set_module_unpatched
@@ -128,6 +129,7 @@ def wrapped_aux_blowfish_function(wrapped, instance, args, kwargs):
128129
@WeakCipher.wrap
129130
def wrapped_rc4_function(wrapped, instance, args, kwargs):
130131
# type: (Callable, Any, Any, Any) -> Any
132+
_set_metric_iast_executed_sink(WeakCipher.vulnerability_type)
131133
WeakCipher.report(
132134
evidence_value="RC4",
133135
)
@@ -139,6 +141,7 @@ def wrapped_function(wrapped, instance, args, kwargs):
139141
# type: (Callable, Any, Any, Any) -> Any
140142
if hasattr(instance, "_dd_weakcipher_algorithm"):
141143
evidence = instance._dd_weakcipher_algorithm + "_" + str(instance.__class__.__name__)
144+
_set_metric_iast_executed_sink(WeakCipher.vulnerability_type)
142145
WeakCipher.report(
143146
evidence_value=evidence,
144147
)
@@ -151,6 +154,7 @@ def wrapped_cryptography_function(wrapped, instance, args, kwargs):
151154
# type: (Callable, Any, Any, Any) -> Any
152155
algorithm_name = instance.algorithm.name.lower()
153156
if algorithm_name in get_weak_cipher_algorithms():
157+
_set_metric_iast_executed_sink(WeakCipher.vulnerability_type)
154158
WeakCipher.report(
155159
evidence_value=algorithm_name,
156160
)

ddtrace/appsec/_iast/taint_sinks/weak_hash.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from ddtrace.internal.logger import get_logger
66

77
from .. import oce
8+
from .._metrics import _set_metric_iast_executed_sink
89
from .._metrics import _set_metric_iast_instrumented_sink
910
from .._patch import set_and_check_module_is_patched
1011
from .._patch import set_module_unpatched
@@ -126,6 +127,7 @@ def patch():
126127
def wrapped_digest_function(wrapped, instance, args, kwargs):
127128
# type: (Callable, Any, Any, Any) -> Any
128129
if instance.name.lower() in get_weak_hash_algorithms():
130+
_set_metric_iast_executed_sink(WeakHash.vulnerability_type)
129131
WeakHash.report(
130132
evidence_value=instance.name,
131133
)
@@ -148,6 +150,7 @@ def wrapped_sha1_function(wrapped, instance, args, kwargs):
148150
def wrapped_new_function(wrapped, instance, args, kwargs):
149151
# type: (Callable, Any, Any, Any) -> Any
150152
if args[0].lower() in get_weak_hash_algorithms():
153+
_set_metric_iast_executed_sink(WeakHash.vulnerability_type)
151154
WeakHash.report(
152155
evidence_value=args[0].lower(),
153156
)
@@ -156,6 +159,7 @@ def wrapped_new_function(wrapped, instance, args, kwargs):
156159

157160
def wrapped_function(wrapped, evidence, instance, args, kwargs):
158161
# type: (Callable, str, Any, Any, Any) -> Any
162+
_set_metric_iast_executed_sink(WeakHash.vulnerability_type)
159163
WeakHash.report(
160164
evidence_value=evidence,
161165
)

ddtrace/contrib/dbapi/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,11 @@ def _trace_method(self, method, name, resource, extra_tags, dbm_propagator, *arg
110110

111111
if _is_iast_enabled():
112112
try:
113+
from ddtrace.appsec._iast._metrics import _set_metric_iast_executed_sink
113114
from ddtrace.appsec._iast._taint_utils import check_tainted_args
114115
from ddtrace.appsec._iast.taint_sinks.sql_injection import SqlInjection
115116

117+
_set_metric_iast_executed_sink(SqlInjection.vulnerability_type)
116118
if check_tainted_args(args, kwargs, pin.tracer, self._self_config.integration_name, method):
117119
SqlInjection.report(evidence_value=args[0])
118120
except Exception:

ddtrace/contrib/dbapi_async/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,11 @@ async def _trace_method(self, method, name, resource, extra_tags, dbm_propagator
7373
s.set_tag_str(SPAN_KIND, SpanKind.CLIENT)
7474

7575
if _is_iast_enabled():
76+
from ddtrace.appsec._iast._metrics import _set_metric_iast_executed_sink
7677
from ddtrace.appsec._iast._taint_utils import check_tainted_args
7778
from ddtrace.appsec._iast.taint_sinks.sql_injection import SqlInjection
7879

80+
_set_metric_iast_executed_sink(SqlInjection.vulnerability_type)
7981
if check_tainted_args(args, kwargs, pin.tracer, self._self_config.integration_name, method):
8082
SqlInjection.report(evidence_value=args[0])
8183

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
IAST: fix executed sink telemetry metric as it is not really linked to vulnerability report.

0 commit comments

Comments
 (0)