Skip to content

Commit 11f31c9

Browse files
authored
chore(appsec): move iast sqli logic (#13748)
Remove appsec logic from contrib files ## 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 b152e1c commit 11f31c9

File tree

8 files changed

+47
-61
lines changed

8 files changed

+47
-61
lines changed

ddtrace/appsec/_iast/_listener.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from ddtrace.appsec._iast._handlers import _on_wsgi_environ
1515
from ddtrace.appsec._iast._iast_request_context import _iast_end_request
1616
from ddtrace.appsec._iast._langchain import langchain_listen
17+
from ddtrace.appsec._iast.taint_sinks.sql_injection import _on_report_sqli
1718
from ddtrace.internal import core
1819

1920

@@ -39,6 +40,9 @@ def iast_listen():
3940
core.on("context.ended.wsgi.__call__", _iast_end_request)
4041
core.on("context.ended.asgi.__call__", _iast_end_request)
4142

43+
# Sink points
44+
core.on("db_query_check", _on_report_sqli)
45+
4246
langchain_listen(core)
4347

4448

ddtrace/appsec/_iast/taint_sinks/sql_injection.py

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,3 @@
1-
from typing import Any
2-
from typing import Callable
3-
from typing import Dict
4-
from typing import Text
5-
from typing import Tuple
6-
71
from ddtrace.appsec._constants import IAST
82
from ddtrace.appsec._constants import IAST_SPAN_TAGS
93
from ddtrace.appsec._iast._logs import iast_error
@@ -22,9 +16,7 @@ class SqlInjection(VulnerabilityBase):
2216
secure_mark = VulnerabilityType.SQL_INJECTION
2317

2418

25-
def check_and_report_sqli(
26-
args: Tuple[Text, ...], kwargs: Dict[str, Any], integration_name: Text, method: Callable[..., Any]
27-
) -> bool:
19+
def _on_report_sqli(*args, **kwargs) -> bool:
2820
"""Check for SQL injection vulnerabilities in database operations and report them.
2921
3022
This function analyzes database operation arguments for potential SQL injection
@@ -36,18 +28,27 @@ def check_and_report_sqli(
3628
This function is part of the IAST (Interactive Application Security Testing)
3729
system and is used to detect potential SQL injection vulnerabilities at runtime.
3830
"""
31+
3932
reported = False
4033
try:
41-
if supported_dbapi_integration(integration_name) and method.__name__ == "execute":
42-
if len(args) and args[0] and isinstance(args[0], IAST.TEXT_TYPES) and asm_config.is_iast_request_enabled:
43-
if SqlInjection.has_quota() and SqlInjection.is_tainted_pyobject(args[0]):
44-
SqlInjection.report(evidence_value=args[0], dialect=integration_name)
45-
reported = True
46-
47-
# Reports Span Metrics
48-
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, SqlInjection.vulnerability_type)
49-
# Report Telemetry Metrics
50-
_set_metric_iast_executed_sink(SqlInjection.vulnerability_type)
34+
if asm_config._iast_enabled:
35+
query_args, kwargs, integration_name, method = args
36+
37+
if supported_dbapi_integration(integration_name) and method.__name__ == "execute":
38+
if (
39+
len(query_args)
40+
and query_args[0]
41+
and isinstance(query_args[0], IAST.TEXT_TYPES)
42+
and asm_config.is_iast_request_enabled
43+
):
44+
if SqlInjection.has_quota() and SqlInjection.is_tainted_pyobject(query_args[0]):
45+
SqlInjection.report(evidence_value=query_args[0], dialect=integration_name)
46+
reported = True
47+
48+
# Reports Span Metrics
49+
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, SqlInjection.vulnerability_type)
50+
# Report Telemetry Metrics
51+
_set_metric_iast_executed_sink(SqlInjection.vulnerability_type)
5152
except Exception as e:
5253
iast_error(f"propagation::sink_point::Error in check_and_report_sqli. {e}")
5354
return reported

ddtrace/contrib/dbapi.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from ddtrace.internal.logger import get_logger
1010
from ddtrace.internal.utils import ArgumentError
1111
from ddtrace.internal.utils import get_argument_value
12-
from ddtrace.settings.asm import config as asm_config
1312

1413
from ..constants import _SPAN_MEASURED_KEY
1514
from ..constants import SPAN_KIND
@@ -100,10 +99,9 @@ def _trace_method(self, method, name, resource, extra_tags, dbm_propagator, *arg
10099
# set span.kind to the type of request being performed
101100
s.set_tag_str(SPAN_KIND, SpanKind.CLIENT)
102101

103-
if asm_config._iast_enabled:
104-
from ddtrace.appsec._iast.taint_sinks.sql_injection import check_and_report_sqli
102+
# Security and IAST validations
103+
core.dispatch("db_query_check", (args, kwargs, self._self_config.integration_name, method))
105104

106-
check_and_report_sqli(args, kwargs, self._self_config.integration_name, method)
107105
# dispatch DBM
108106
if dbm_propagator:
109107
# this check is necessary to prevent fetch methods from trying to add dbm propagation

ddtrace/contrib/dbapi_async.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
from ddtrace.internal.logger import get_logger
55
from ddtrace.internal.utils import ArgumentError
66
from ddtrace.internal.utils import get_argument_value
7-
from ddtrace.settings.asm import config as asm_config
87

98
from ..constants import _SPAN_MEASURED_KEY
109
from ..constants import SPAN_KIND
@@ -75,16 +74,14 @@ async def _trace_method(self, method, name, resource, extra_tags, dbm_propagator
7574
# set span.kind to the type of request being performed
7675
s.set_tag_str(SPAN_KIND, SpanKind.CLIENT)
7776

78-
if asm_config._iast_enabled:
79-
from ddtrace.appsec._iast.taint_sinks.sql_injection import check_and_report_sqli
80-
81-
check_and_report_sqli(args, kwargs, self._self_config.integration_name, method)
77+
# Security and IAST validations
78+
core.dispatch("db_query_check", (args, kwargs, self._self_config.integration_name, method))
8279

8380
# dispatch DBM
8481
if dbm_propagator:
8582
# this check is necessary to prevent fetch methods from trying to add dbm propagation
8683
result = core.dispatch_with_results(
87-
f"{self._self_config.integration_name}.execute", [self._self_config, s, args, kwargs]
84+
f"{self._self_config.integration_name}.execute", (self._self_config, s, args, kwargs)
8885
).result
8986
if result:
9087
s, args, kwargs = result.value

tests/appsec/iast/taint_sinks/test_sql_injection.py

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from ddtrace.appsec._iast._taint_tracking import OriginType
77
from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject
88
from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
9-
from ddtrace.appsec._iast.taint_sinks.sql_injection import check_and_report_sqli
9+
from ddtrace.appsec._iast.taint_sinks.sql_injection import _on_report_sqli
1010

1111

1212
def test_checked_tainted_args(iast_context_deduplication_enabled):
@@ -21,45 +21,24 @@ def test_checked_tainted_args(iast_context_deduplication_enabled):
2121
untainted_arg = "gallahad the pure"
2222

2323
# Returns False: Untainted first argument
24-
assert not check_and_report_sqli(
25-
args=(untainted_arg,), kwargs=None, integration_name="sqlite", method=cursor.execute
26-
)
24+
assert not _on_report_sqli((untainted_arg,), None, "sqlite", cursor.execute)
2725

2826
# Returns False: Untainted first argument
29-
assert not check_and_report_sqli(
30-
args=(untainted_arg, tainted_arg), kwargs=None, integration_name="sqlite", method=cursor.execute
31-
)
32-
27+
assert not _on_report_sqli((untainted_arg, tainted_arg), None, "sqlite", cursor.execute)
3328
# Returns False: Integration name not in list
34-
assert not check_and_report_sqli(
35-
args=(tainted_arg,),
36-
kwargs=None,
37-
integration_name="nosqlite",
38-
method=cursor.execute,
39-
)
29+
assert not _on_report_sqli((tainted_arg,), None, "nosqlite", cursor.execute)
4030

4131
# Returns False: Wrong function name
42-
assert not check_and_report_sqli(
43-
args=(tainted_arg,),
44-
kwargs=None,
45-
integration_name="sqlite",
46-
method=cursor.executemany,
47-
)
32+
assert not _on_report_sqli((tainted_arg,), None, "sqlite", cursor.executemany)
4833

4934
# Returns True:
50-
assert check_and_report_sqli(
51-
args=(tainted_arg, untainted_arg), kwargs=None, integration_name="sqlite", method=cursor.execute
52-
)
35+
assert _on_report_sqli((tainted_arg, untainted_arg), None, "sqlite", cursor.execute)
5336

5437
# Returns True:
55-
assert check_and_report_sqli(
56-
args=(tainted_arg, untainted_arg), kwargs=None, integration_name="mysql", method=cursor.execute
57-
)
38+
assert _on_report_sqli((tainted_arg, untainted_arg), None, "mysql", cursor.execute)
5839

5940
# Returns False: No more QUOTA
60-
assert not check_and_report_sqli(
61-
args=(tainted_arg, untainted_arg), kwargs=None, integration_name="psycopg", method=cursor.execute
62-
)
41+
assert not _on_report_sqli((tainted_arg, untainted_arg), None, "psycopg", cursor.execute)
6342

6443

6544
@pytest.mark.parametrize(
@@ -116,7 +95,7 @@ def test_check_and_report_sqli_metrics(args, integration_name, expected_result,
11695
"ddtrace.appsec._iast.taint_sinks.sql_injection._set_metric_iast_executed_sink"
11796
) as mock_set_metric:
11897
# Call with tainted argument that should trigger metrics
119-
result = check_and_report_sqli(args=args, kwargs={}, integration_name=integration_name, method=cursor.execute)
98+
result = _on_report_sqli(args, {}, integration_name, cursor.execute)
12099

121100
assert result is expected_result
122101
mock_increment.assert_called_once()
@@ -154,7 +133,7 @@ def test_check_and_report_sqli_no_metrics(args, integration_name, iast_context_d
154133
"ddtrace.appsec._iast.taint_sinks.sql_injection._set_metric_iast_executed_sink"
155134
) as mock_set_metric:
156135
# Call with untainted argument that should not trigger metrics
157-
result = check_and_report_sqli(args=args, kwargs={}, integration_name=integration_name, method=cursor.execute)
136+
result = _on_report_sqli(args, {}, integration_name, cursor.execute)
158137

159138
assert result is False
160139
mock_increment.assert_not_called()

tests/contrib/dbapi/test_dbapi_appsec.py renamed to tests/appsec/iast/taint_sinks/test_sql_injection_dbapi.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
import mock
1+
from unittest import mock
2+
23
import pytest
34

5+
from ddtrace.appsec._iast import load_iast
46
from ddtrace.appsec._iast._overhead_control_engine import oce
57
from ddtrace.contrib.dbapi import TracedCursor
68
from ddtrace.settings._config import Config
@@ -16,6 +18,7 @@
1618
class TestTracedCursor(TracerTestCase):
1719
def setUp(self):
1820
super(TestTracedCursor, self).setUp()
21+
load_iast()
1922
with override_global_config(
2023
dict(
2124
_iast_enabled=True,

tests/appsec/integrations/django_tests/conftest.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import pytest
66

77
from ddtrace.appsec._iast import enable_iast_propagation
8+
from ddtrace.appsec._iast import load_iast
89
from ddtrace.appsec._iast.main import patch_iast
910
from ddtrace.contrib.internal.django.patch import patch as django_patch
1011
from ddtrace.contrib.internal.requests.patch import patch as requests_patch
@@ -32,6 +33,7 @@ def pytest_configure():
3233
), override_env(dict(_DD_IAST_PATCH_MODULES="tests.appsec.integrations")):
3334
settings.DEBUG = False
3435
patch_iast()
36+
load_iast()
3537
requests_patch()
3638
django_patch()
3739
enable_iast_propagation()

tests/appsec/integrations/packages_tests/test_iast_sql_injection.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import pytest
22

33
from ddtrace import patch
4+
from ddtrace.appsec._iast import load_iast
45
from ddtrace.appsec._iast._taint_tracking import OriginType
56
from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject
67
from ddtrace.appsec._iast._taint_tracking._taint_objects_base import is_pyobject_tainted
@@ -37,6 +38,7 @@
3738

3839
def setup_module():
3940
patch(pymysql=True, mysqldb=True)
41+
load_iast()
4042

4143

4244
@pytest.mark.parametrize("fixture_path,fixture_module", DDBBS)

0 commit comments

Comments
 (0)