Skip to content

Commit 25684ea

Browse files
authored
feat(iast): add secure marks core and sanitize command injection (#13017)
## Overview This PR implements the secure marks functionality for IAST, focusing on command injection sanitization. It adds core functionality to mark strings as secure after they've been validated or sanitized by security functions. ## Motivation - Improve IAST vulnerability detection accuracy by tracking sanitized inputs - Reduce false positives by properly marking strings that have been secured - Implement command injection protection as part of the secure marks system ## Changes - Added new `secure_marks` package in `ddtrace/appsec/_iast/` - Implemented validators and sanitizers for command injection protection - Added support for common security functions (shlex.quote, shlex.split, django.utils.shlex.quote) - Updated taint tracking to support secure marks - Added comprehensive test coverage for the new functionality ## Testing Strategy - Unit tests for validators and sanitizers - Integration tests with common security functions - C++ tests for taint range secure marks functionality - Performance impact validated (import time improved by ~1.4ms) ## 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 3a30651 commit 25684ea

File tree

18 files changed

+644
-277
lines changed

18 files changed

+644
-277
lines changed

ddtrace/appsec/_iast/_ast/iastpatch.c

+55-217
Original file line numberDiff line numberDiff line change
@@ -167,224 +167,62 @@ static const char* static_denylist[] = { "django.apps.config.",
167167
"django_filters.utils.",
168168
"django_filters.widgets." };
169169

170-
static size_t static_stdlib_count = 216;
170+
static size_t static_stdlib_count = 215;
171171
static const char* static_stdlib_denylist[] = {
172-
"__future__",
173-
"_ast",
174-
"_compression",
175-
"_thread",
176-
"abc",
177-
"aifc",
178-
"argparse",
179-
"array",
180-
"ast",
181-
"asynchat",
182-
"asyncio",
183-
"asyncore",
184-
"atexit",
185-
"audioop",
186-
"base64",
187-
"bdb",
188-
"binascii",
189-
"bisect",
190-
"builtins",
191-
"bz2",
192-
"cProfile",
193-
"calendar",
194-
"cgi",
195-
"cgitb",
196-
"chunk",
197-
"cmath",
198-
"cmd",
199-
"code",
200-
"codecs",
201-
"codeop",
202-
"collections",
203-
"colorsys",
204-
"compileall",
205-
"concurrent",
206-
"configparser",
207-
"contextlib",
208-
"contextvars",
209-
"copy",
210-
"copyreg",
211-
"crypt",
212-
"csv",
213-
"ctypes",
214-
"curses",
215-
"dataclasses",
216-
"datetime",
217-
"dbm",
218-
"decimal",
219-
"difflib",
220-
"dis",
221-
"distutils",
222-
"doctest",
223-
"email",
224-
"encodings",
225-
"ensurepip",
226-
"enum",
227-
"errno",
228-
"faulthandler",
229-
"fcntl",
230-
"filecmp",
231-
"fileinput",
232-
"fnmatch",
233-
"fractions",
234-
"ftplib",
235-
"functools",
236-
"gc",
237-
"getopt",
238-
"getpass",
239-
"gettext",
240-
"glob",
241-
"graphlib",
242-
"grp",
243-
"gzip",
244-
"hashlib",
245-
"heapq",
246-
"hmac",
247-
"html",
248-
"http",
249-
"idlelib",
250-
"imaplib",
251-
"imghdr",
252-
"imp",
253-
"importlib",
254-
"inspect",
255-
"io",
256-
"_io",
257-
"ipaddress",
258-
"itertools",
259-
"json",
260-
"keyword",
261-
"lib2to3",
262-
"linecache",
263-
"locale",
264-
"logging",
265-
"lzma",
266-
"mailbox",
267-
"mailcap",
268-
"marshal",
269-
"math",
270-
"mimetypes",
271-
"mmap",
272-
"modulefinder",
273-
"msilib",
274-
"msvcrt",
275-
"multiprocessing",
276-
"netrc",
277-
"nis",
278-
"nntplib",
279-
"ntpath",
280-
"numbers",
281-
"opcode",
282-
"operator",
283-
"optparse",
284-
"os",
285-
"ossaudiodev",
286-
"pathlib",
287-
"pdb",
288-
"pickle",
289-
"pickletools",
290-
"pipes",
291-
"pkgutil",
292-
"platform",
293-
"plistlib",
294-
"poplib",
295-
"posix",
296-
"posixpath",
297-
"pprint",
298-
"profile",
299-
"pstats",
300-
"pty",
301-
"pwd",
302-
"py_compile",
303-
"pyclbr",
304-
"pydoc",
305-
"queue",
306-
"quopri",
307-
"random",
308-
"re",
309-
"readline",
310-
"reprlib",
311-
"resource",
312-
"rlcompleter",
313-
"runpy",
314-
"sched",
315-
"secrets",
316-
"select",
317-
"selectors",
318-
"shelve",
319-
"shlex",
320-
"shutil",
321-
"signal",
322-
"site",
323-
"smtpd",
324-
"smtplib",
325-
"sndhdr",
326-
"socket",
327-
"socketserver",
328-
"spwd",
329-
"sqlite3",
330-
"sre",
331-
"sre_compile",
332-
"sre_constants",
333-
"sre_parse",
334-
"ssl",
335-
"stat",
336-
"statistics",
337-
"string",
338-
"stringprep",
339-
"struct",
340-
"subprocess",
341-
"sunau",
342-
"symtable",
343-
"sys",
344-
"sysconfig",
345-
"syslog",
346-
"tabnanny",
347-
"tarfile",
348-
"telnetlib",
349-
"tempfile",
350-
"termios",
351-
"test",
352-
"textwrap",
353-
"threading",
354-
"time",
355-
"timeit",
356-
"tkinter",
357-
"token",
358-
"tokenize",
359-
"tomllib",
360-
"trace",
361-
"traceback",
362-
"tracemalloc",
363-
"tty",
364-
"turtle",
365-
"turtledemo",
366-
"types",
367-
"typing",
368-
"unicodedata",
369-
"unittest",
370-
"uu",
371-
"uuid",
372-
"venv",
373-
"warnings",
374-
"wave",
375-
"weakref",
376-
"webbrowser",
377-
"winreg",
378-
"winsound",
379-
"wsgiref",
380-
"xdrlib",
381-
"xml",
382-
"xmlrpc",
383-
"zipapp",
384-
"zipfile",
385-
"zipimport",
386-
"zlib",
387-
"zoneinfo",
172+
"__future__", "_ast", "_compression", "_thread",
173+
"abc", "aifc", "argparse", "array",
174+
"ast", "asynchat", "asyncio", "asyncore",
175+
"atexit", "audioop", "base64", "bdb",
176+
"binascii", "bisect", "builtins", "bz2",
177+
"cProfile", "calendar", "cgi", "cgitb",
178+
"chunk", "cmath", "cmd", "code",
179+
"codecs", "codeop", "collections", "colorsys",
180+
"compileall", "concurrent", "configparser", "contextlib",
181+
"contextvars", "copy", "copyreg", "crypt",
182+
"csv", "ctypes", "curses", "dataclasses",
183+
"datetime", "dbm", "decimal", "difflib",
184+
"dis", "distutils", "doctest", "email",
185+
"encodings", "ensurepip", "enum", "errno",
186+
"faulthandler", "fcntl", "filecmp", "fileinput",
187+
"fnmatch", "fractions", "ftplib", "functools",
188+
"gc", "getopt", "getpass", "gettext",
189+
"glob", "graphlib", "grp", "gzip",
190+
"hashlib", "heapq", "hmac", "html",
191+
"http", "idlelib", "imaplib", "imghdr",
192+
"imp", "importlib", "inspect", "io",
193+
"_io", "ipaddress", "itertools", "json",
194+
"keyword", "lib2to3", "linecache", "locale",
195+
"logging", "lzma", "mailbox", "mailcap",
196+
"marshal", "math", "mimetypes", "mmap",
197+
"modulefinder", "msilib", "msvcrt", "multiprocessing",
198+
"netrc", "nis", "nntplib", "ntpath",
199+
"numbers", "opcode", "operator", "optparse",
200+
"os", "ossaudiodev", "pathlib", "pdb",
201+
"pickle", "pickletools", "pipes", "pkgutil",
202+
"platform", "plistlib", "poplib", "posix",
203+
"posixpath", "pprint", "profile", "pstats",
204+
"pty", "pwd", "py_compile", "pyclbr",
205+
"pydoc", "queue", "quopri", "random",
206+
"re", "readline", "reprlib", "resource",
207+
"rlcompleter", "runpy", "sched", "secrets",
208+
"select", "selectors", "shelve", "shutil",
209+
"signal", "site", "smtpd", "smtplib",
210+
"sndhdr", "socket", "socketserver", "spwd",
211+
"sqlite3", "sre", "sre_compile", "sre_constants",
212+
"sre_parse", "ssl", "stat", "statistics",
213+
"string", "stringprep", "struct", "subprocess",
214+
"sunau", "symtable", "sys", "sysconfig",
215+
"syslog", "tabnanny", "tarfile", "telnetlib",
216+
"tempfile", "termios", "test", "textwrap",
217+
"threading", "time", "timeit", "tkinter",
218+
"token", "tokenize", "tomllib", "trace",
219+
"traceback", "tracemalloc", "tty", "turtle",
220+
"turtledemo", "types", "typing", "unicodedata",
221+
"unittest", "uu", "uuid", "venv",
222+
"warnings", "wave", "weakref", "webbrowser",
223+
"winreg", "winsound", "wsgiref", "xdrlib",
224+
"xml", "xmlrpc", "zipapp", "zipfile",
225+
"zipimport", "zlib", "zoneinfo",
388226
};
389227

390228
/* --- Helper function: str_in_list ---

ddtrace/appsec/_iast/_handlers.py

+11-5
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
from ddtrace.appsec._iast._iast_request_context import get_iast_stacktrace_reported
88
from ddtrace.appsec._iast._iast_request_context import set_iast_stacktrace_reported
9+
from ddtrace.appsec._iast._logs import iast_instrumentation_wrapt_debug_log
10+
from ddtrace.appsec._iast._logs import iast_propagation_listener_log_log
911
from ddtrace.appsec._iast._metrics import _set_metric_iast_instrumented_source
1012
from ddtrace.appsec._iast._patch import _iast_instrument_starlette_request
1113
from ddtrace.appsec._iast._patch import _iast_instrument_starlette_request_body
@@ -15,14 +17,12 @@
1517
from ddtrace.appsec._iast._taint_tracking import OriginType
1618
from ddtrace.appsec._iast._taint_tracking import origin_to_str
1719
from ddtrace.appsec._iast._taint_tracking._taint_objects import is_pyobject_tainted
20+
from ddtrace.appsec._iast._taint_tracking._taint_objects import taint_pyobject
1821
from ddtrace.appsec._iast._taint_utils import taint_structure
22+
from ddtrace.appsec._iast.secure_marks.sanitizers import cmdi_sanitizer
1923
from ddtrace.internal.logger import get_logger
2024
from ddtrace.settings.asm import config as asm_config
2125

22-
from ._logs import iast_instrumentation_wrapt_debug_log
23-
from ._logs import iast_propagation_listener_log_log
24-
from ._taint_tracking._taint_objects import taint_pyobject
25-
2626

2727
MessageMapContainer = None
2828
try:
@@ -55,6 +55,11 @@ def _on_request_init(wrapped, instance, args, kwargs):
5555

5656

5757
def _on_flask_patch(flask_version):
58+
"""Handle Flask framework patch event.
59+
60+
Args:
61+
flask_version: The version tuple of Flask being patched
62+
"""
5863
if asm_config._iast_enabled:
5964
try:
6065
try_wrap_function_wrapper(
@@ -148,6 +153,7 @@ def _on_wsgi_environ(wrapped, _instance, args, kwargs):
148153

149154

150155
def _on_django_patch():
156+
"""Handle Django framework patch event."""
151157
if asm_config._iast_enabled:
152158
try:
153159
when_imported("django.http.request")(
@@ -157,7 +163,7 @@ def _on_django_patch():
157163
functools.partial(if_iast_taint_returned_object_for, OriginType.PARAMETER),
158164
)
159165
)
160-
166+
try_wrap_function_wrapper("django.utils.shlex", "quote", cmdi_sanitizer)
161167
# we instrument those sources on _on_django_func_wrapped
162168
_set_metric_iast_instrumented_source(OriginType.HEADER_NAME)
163169
_set_metric_iast_instrumented_source(OriginType.HEADER)

ddtrace/appsec/_iast/_patch_modules.py

+47
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
from wrapt.importer import when_imported
22

3+
from ddtrace.appsec._common_module_patches import try_wrap_function_wrapper
4+
from ddtrace.appsec._iast.secure_marks.sanitizers import cmdi_sanitizer
5+
from ddtrace.appsec._iast.secure_marks.sanitizers import sqli_sanitizer
6+
37

48
IAST_PATCH = {
59
"code_injection": True,
@@ -23,4 +27,47 @@ def patch_iast(patch_modules=IAST_PATCH):
2327
for module in (m for m, e in patch_modules.items() if e):
2428
when_imported("hashlib")(_on_import_factory(module, "ddtrace.appsec._iast.taint_sinks.%s", raise_errors=False))
2529

30+
when_imported("shlex")(
31+
lambda _: try_wrap_function_wrapper(
32+
"shlex",
33+
"quote",
34+
cmdi_sanitizer,
35+
)
36+
)
37+
when_imported("shlex")(
38+
lambda _: try_wrap_function_wrapper(
39+
"shlex",
40+
"split",
41+
cmdi_sanitizer,
42+
)
43+
)
44+
when_imported("psycopg2.adapt")(
45+
lambda _: try_wrap_function_wrapper(
46+
"psycopg2.adapt",
47+
"quote_ident",
48+
sqli_sanitizer,
49+
)
50+
)
51+
when_imported("psycopg2.extensions")(
52+
lambda _: try_wrap_function_wrapper(
53+
"psycopg2.extensions",
54+
"quote_ident",
55+
sqli_sanitizer,
56+
)
57+
)
58+
# TODO: APPSEC-56947 task
59+
# when_imported("mysql.connector.conversion")(
60+
# lambda _: try_wrap_function_wrapper(
61+
# "mysql.connector.conversion",
62+
# "MySQLConverter.escape",
63+
# sqli_sanitizer,
64+
# )
65+
# )
66+
# when_imported("werkzeug.utils")(
67+
# lambda _: try_wrap_function_wrapper(
68+
# "werkzeug.utils",
69+
# "secure_filename",
70+
# path_traversal_sanitizer,
71+
# )
72+
# )
2673
when_imported("json")(_on_import_factory("json_tainting", "ddtrace.appsec._iast._patches.%s", raise_errors=False))

0 commit comments

Comments
 (0)