Skip to content

Commit eff3155

Browse files
avara1986gnufede
andauthored
feat(iast): xss vulnerability for django applications (#12116)
XSS vulnerability detection. System tests (merge after this PR): DataDog/system-tests#3923 ## 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: Federico Mon <[email protected]>
1 parent 56a6ca2 commit eff3155

File tree

17 files changed

+736
-511
lines changed

17 files changed

+736
-511
lines changed

Diff for: ddtrace/appsec/_iast/_evidence_redaction/_sensitive_handler.py

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from ..constants import VULN_HEADER_INJECTION
1111
from ..constants import VULN_SQL_INJECTION
1212
from ..constants import VULN_SSRF
13+
from ..constants import VULN_XSS
1314
from .command_injection_sensitive_analyzer import command_injection_sensitive_analyzer
1415
from .default_sensitive_analyzer import default_sensitive_analyzer
1516
from .header_injection_sensitive_analyzer import header_injection_sensitive_analyzer
@@ -45,6 +46,7 @@ def __init__(self):
4546
VULN_SQL_INJECTION: sql_sensitive_analyzer,
4647
VULN_SSRF: url_sensitive_analyzer,
4748
VULN_HEADER_INJECTION: header_injection_sensitive_analyzer,
49+
VULN_XSS: default_sensitive_analyzer,
4850
VULN_CODE_INJECTION: default_sensitive_analyzer,
4951
}
5052

Diff for: ddtrace/appsec/_iast/_patch_modules.py

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"header_injection": True,
88
"weak_cipher": True,
99
"weak_hash": True,
10+
"xss": True,
1011
}
1112

1213

Diff for: ddtrace/appsec/_iast/constants.py

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
VULN_CMDI = "COMMAND_INJECTION"
1515
VULN_HEADER_INJECTION = "HEADER_INJECTION"
1616
VULN_CODE_INJECTION = "CODE_INJECTION"
17+
VULN_XSS = "XSS"
1718
VULN_SSRF = "SSRF"
1819
VULN_STACKTRACE_LEAK = "STACKTRACE_LEAK"
1920

Diff for: ddtrace/appsec/_iast/taint_sinks/xss.py

+78
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
from typing import Text
2+
3+
from ddtrace.appsec._common_module_patches import try_unwrap
4+
from ddtrace.appsec._constants import IAST_SPAN_TAGS
5+
from ddtrace.appsec._iast import oce
6+
from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled
7+
from ddtrace.appsec._iast._metrics import _set_metric_iast_executed_sink
8+
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_sink
9+
from ddtrace.appsec._iast._metrics import increment_iast_span_metric
10+
from ddtrace.appsec._iast._patch import set_and_check_module_is_patched
11+
from ddtrace.appsec._iast._patch import set_module_unpatched
12+
from ddtrace.appsec._iast._patch import try_wrap_function_wrapper
13+
from ddtrace.appsec._iast._taint_tracking._taint_objects import is_pyobject_tainted
14+
from ddtrace.appsec._iast.constants import VULN_XSS
15+
from ddtrace.appsec._iast.taint_sinks._base import VulnerabilityBase
16+
from ddtrace.internal.logger import get_logger
17+
from ddtrace.settings.asm import config as asm_config
18+
19+
20+
log = get_logger(__name__)
21+
22+
23+
@oce.register
24+
class XSS(VulnerabilityBase):
25+
vulnerability_type = VULN_XSS
26+
27+
28+
def get_version() -> Text:
29+
return ""
30+
31+
32+
def patch():
33+
if not asm_config._iast_enabled:
34+
return
35+
36+
if not set_and_check_module_is_patched("flask", default_attr="_datadog_xss_patch"):
37+
return
38+
if not set_and_check_module_is_patched("django", default_attr="_datadog_xss_patch"):
39+
return
40+
if not set_and_check_module_is_patched("fastapi", default_attr="_datadog_xss_patch"):
41+
return
42+
43+
try_wrap_function_wrapper(
44+
"django.utils.safestring",
45+
"mark_safe",
46+
_iast_django_xss,
47+
)
48+
49+
try_wrap_function_wrapper(
50+
"django.template.defaultfilters",
51+
"mark_safe",
52+
_iast_django_xss,
53+
)
54+
55+
_set_metric_iast_instrumented_sink(VULN_XSS)
56+
57+
58+
def unpatch():
59+
try_unwrap("django.utils.safestring", "mark_safe")
60+
try_unwrap("django.template.defaultfilters", "mark_safe")
61+
62+
set_module_unpatched("flask", default_attr="_datadog_xss_patch")
63+
set_module_unpatched("django", default_attr="_datadog_xss_patch")
64+
set_module_unpatched("fastapi", default_attr="_datadog_xss_patch")
65+
66+
67+
def _iast_django_xss(wrapped, instance, args, kwargs):
68+
if args and len(args) >= 1:
69+
_iast_report_xss(args[0])
70+
return wrapped(*args, **kwargs)
71+
72+
73+
def _iast_report_xss(code_string: Text):
74+
increment_iast_span_metric(IAST_SPAN_TAGS.TELEMETRY_EXECUTED_SINK, XSS.vulnerability_type)
75+
_set_metric_iast_executed_sink(XSS.vulnerability_type)
76+
if is_iast_request_enabled():
77+
if is_pyobject_tainted(code_string):
78+
XSS.report(evidence_value=code_string)

Diff for: docker-compose.yml

-1
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,6 @@ services:
198198
ports:
199199
- "127.0.0.1:8321:8321"
200200
environment:
201-
- DD_APPSEC_ENABLED=true
202201
- DD_IAST_ENABLED=true
203202
- DD_IAST_REQUEST_SAMPLING=100
204203
- DD_IAST_VULNERABILITIES_PER_REQUEST=100
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
features:
3+
- |
4+
Code Security (IAST): XSS detection for Django applications,
5+
which will be displayed on your DataDog Vulnerability Explorer dashboard.
6+
See the `Application Vulnerability Management <https://docs.datadoghq.com/security/application_security/vulnerability_management/>`_ documentation for more information about this feature.

Diff for: tests/appsec/iast/taint_sinks/_taint_sinks_utils.py

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ def get_parametrize(vuln_type, ignore_list=None):
2222
"$1 - Tainted range based redaction - multiple ranges",
2323
"Redacted source that needs to be truncated",
2424
"Query with single quoted string literal and null source",
25+
"No redacted that needs to be truncated - whole text",
2526
):
2627
continue
2728

Diff for: tests/appsec/iast/taint_sinks/test_xss_redacted.py

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import os
2+
3+
import pytest
4+
5+
from ddtrace.appsec._iast._taint_tracking import origin_to_str
6+
from ddtrace.appsec._iast._taint_tracking import str_to_origin
7+
from ddtrace.appsec._iast.constants import VULN_XSS
8+
from ddtrace.appsec._iast.taint_sinks.xss import XSS
9+
from tests.appsec.iast.taint_sinks._taint_sinks_utils import _taint_pyobject_multiranges
10+
from tests.appsec.iast.taint_sinks._taint_sinks_utils import get_parametrize
11+
from tests.appsec.iast.taint_sinks.conftest import _get_iast_data
12+
13+
14+
ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
15+
16+
17+
@pytest.mark.parametrize(
18+
"evidence_input, sources_expected, vulnerabilities_expected,element", list(get_parametrize(VULN_XSS))
19+
)
20+
def test_xss_redaction_suite(
21+
evidence_input, sources_expected, vulnerabilities_expected, iast_context_defaults, element
22+
):
23+
tainted_object = evidence_input_value = evidence_input.get("value", "")
24+
if evidence_input_value:
25+
tainted_object = _taint_pyobject_multiranges(
26+
evidence_input_value,
27+
[
28+
(
29+
input_ranges["iinfo"]["parameterName"],
30+
input_ranges["iinfo"]["parameterValue"],
31+
str_to_origin(input_ranges["iinfo"]["type"]),
32+
input_ranges["start"],
33+
input_ranges["end"] - input_ranges["start"],
34+
)
35+
for input_ranges in evidence_input.get("ranges", {})
36+
],
37+
)
38+
39+
XSS.report(tainted_object)
40+
41+
data = _get_iast_data()
42+
vulnerability = list(data["vulnerabilities"])[0]
43+
source = list(data["sources"])[0]
44+
source["origin"] = origin_to_str(source["origin"])
45+
46+
assert vulnerability["type"] == VULN_XSS
47+
assert vulnerability["evidence"] == vulnerabilities_expected["evidence"]
48+
assert source == sources_expected

Diff for: tests/appsec/integrations/django_tests/conftest.py

+13-1
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._patch_modules import patch_iast
89
from ddtrace.contrib.internal.django.patch import patch
910
from ddtrace.trace import Pin
1011
from tests.appsec.iast.conftest import _end_iast_context_and_oce
@@ -27,11 +28,22 @@ def pytest_configure():
2728
)
2829
):
2930
settings.DEBUG = False
30-
enable_iast_propagation()
31+
patch_iast()
3132
patch()
33+
enable_iast_propagation()
3234
django.setup()
3335

3436

37+
@pytest.fixture
38+
def debug_mode():
39+
from django.conf import settings
40+
41+
original_debug = settings.DEBUG
42+
settings.DEBUG = True
43+
yield
44+
settings.DEBUG = original_debug
45+
46+
3547
@pytest.fixture
3648
def tracer():
3749
tracer = DummyTracer()

Diff for: tests/appsec/integrations/django_tests/django_app/settings.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
{
4141
"BACKEND": "django.template.backends.django.DjangoTemplates",
4242
"DIRS": [
43-
os.path.join(BASE_DIR, "templates"),
43+
os.path.join(BASE_DIR, "django_app", "templates"),
4444
],
4545
"APP_DIRS": True,
4646
"OPTIONS": {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<html>
2+
<body>
3+
<p>Input: {{ user_input }}</p>
4+
</body>
5+
</html>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<html>
2+
<body>
3+
<p>{% autoescape on %}
4+
{{ user_input }}
5+
{% endautoescape %}</p>
6+
</body>
7+
</html>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<html>
2+
<body>
3+
<p>Input: {{ user_input|safe }}</p>
4+
</body>
5+
</html>

Diff for: tests/appsec/integrations/django_tests/django_app/urls.py

+4
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ def shutdown(request):
7373
handler("appsec/insecure-cookie/test_insecure/$", views.view_insecure_cookies_insecure),
7474
handler("appsec/insecure-cookie/test_secure/$", views.view_insecure_cookies_secure),
7575
handler("appsec/insecure-cookie/test_empty_cookie/$", views.view_insecure_cookies_empty),
76+
handler("appsec/xss/$", views.xss_http_request_parameter_mark_safe),
77+
handler("appsec/xss/secure/$", views.xss_secure),
78+
handler("appsec/xss/safe/$", views.xss_http_request_parameter_template_safe),
79+
handler("appsec/xss/autoscape/$", views.xss_http_request_parameter_autoscape),
7680
path(
7781
"appsec/sqli_http_path_parameter/<str:q_http_path_parameter>/",
7882
views.sqli_http_path_parameter,

Diff for: tests/appsec/integrations/django_tests/django_app/views.py

+30
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
from django.db import connection
99
from django.http import HttpResponse
1010
from django.http import JsonResponse
11+
from django.shortcuts import render
12+
from django.utils.safestring import mark_safe
1113

1214
from ddtrace.appsec import _asm_request_context
1315
from ddtrace.appsec._iast._taint_tracking import OriginType
@@ -68,6 +70,34 @@ def checkuser_view(request, user_id):
6870
return HttpResponse(status=200)
6971

7072

73+
def xss_http_request_parameter_mark_safe(request):
74+
user_input = request.GET.get("input", "")
75+
76+
# label xss_http_request_parameter_mark_safe
77+
return render(request, "index.html", {"user_input": mark_safe(user_input)})
78+
79+
80+
def xss_secure(request):
81+
user_input = request.GET.get("input", "")
82+
83+
# label xss_http_request_parameter_mark_safe
84+
return render(request, "index.html", {"user_input": user_input})
85+
86+
87+
def xss_http_request_parameter_template_safe(request):
88+
user_input = request.GET.get("input", "")
89+
90+
# label xss_http_request_parameter_template_safe
91+
return render(request, "index_safe.html", {"user_input": user_input})
92+
93+
94+
def xss_http_request_parameter_autoscape(request):
95+
user_input = request.GET.get("input", "")
96+
97+
# label xss_http_request_parameter_autoscape
98+
return render(request, "index_autoescape.html", {"user_input": user_input})
99+
100+
71101
def sqli_http_request_parameter(request):
72102
import bcrypt
73103
from django.contrib.auth.hashers import BCryptSHA256PasswordHasher

0 commit comments

Comments
 (0)