feat(iast): weak randomness vulnerability [backport #6912 to 2.0] (#6988)

This change backports #6912 to the 2.0 release branch.

This change also re-enables the system-tests workflow, which had been
temporarily disabled in #6974
to make that pull request mergeable.

The net effect of these git gymnastics is that the diff still waiting to
be merged into 2.0 is as small as possible.

## 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: Munir Abdinur <[email protected]>
Co-authored-by: Tahir H. Butt <[email protected]>
Co-authored-by: Alberto Vara <[email protected]>
Co-authored-by: Federico Mon <[email protected]>

Author: 5 people

.github/workflows/system-tests.yml

@@ -67,9 +67,9 @@ jobs:
         if: needs.needs-run.outputs.outcome == 'success'
         run: ./build.sh
 
-      #- name: Run
-        #  if: needs.needs-run.outputs.outcome == 'success'
-        #run: ./run.sh
+      - name: Run
+        if: needs.needs-run.outputs.outcome == 'success'
+        run: ./run.sh
 
       - name: Run REMOTE_CONFIG_MOCKED_BACKEND_ASM_FEATURES
         if: needs.needs-run.outputs.outcome == 'success'

ddtrace/appsec/_iast/_ast/visitor.py

@@ -1,14 +1,15 @@
 #!/usr/bin/env python3
-
 from _ast import Expr
 from _ast import ImportFrom
 import ast
+import copy
 import sys
 from typing import TYPE_CHECKING
 
 from six import iteritems
 
 from .._metrics import _set_metric_iast_instrumented_propagation
+from ..constants import DEFAULT_WEAK_RANDOMNESS_FUNCTIONS
 
 
 if TYPE_CHECKING:  # pragma: no cover
@@ -20,6 +21,12 @@
 PY30_37 = sys.version_info >= (3, 0, 0) and sys.version_info < (3, 8, 0)
 PY38_PLUS = sys.version_info >= (3, 8, 0)
 
+CODE_TYPE_FIRST_PARTY = "first_party"
+CODE_TYPE_DD = "datadog"
+CODE_TYPE_SITE_PACKAGES = "site_packages"
+CODE_TYPE_STDLIB = "stdlib"
+TAINT_SINK_FUNCTION_REPLACEMENT = "ddtrace_taint_sinks.ast_funcion"
+
 
 class AstVisitor(ast.NodeTransformer):
     def __init__(
@@ -86,6 +93,26 @@ def __init__(
                 },
                 "django.utils.html": {"": ("format_html", "format_html_join")},
             },
+            # This is a set since all functions will be replaced by taint_sink_functions
+            "taint_sinks": {
+                "weak_randomness": DEFAULT_WEAK_RANDOMNESS_FUNCTIONS,
+                "other": {
+                    "load",
+                    "run",
+                    "path",
+                    "exit",
+                    "sleep",
+                    "socket",
+                },
+                # These explicitly WON'T be replaced by taint_sink_function:
+                "disabled": {
+                    "__new__",
+                    "__init__",
+                    "__dir__",
+                    "__repr__",
+                    "super",
+                },
+            },
         }
         self._sinkpoints_spec = {
             "definitions_module": "ddtrace.appsec._iast.taint_sinks",
@@ -107,6 +134,12 @@ def __init__(
         self._aspect_build_string = self._aspects_spec["operators"]["BUILD_STRING"]
         self.excluded_functions = self._aspects_spec["excluded_from_patching"].get(self.module_name, {})
 
+        # Sink points
+        self._taint_sink_replace_random = self._aspects_spec["taint_sinks"]["weak_randomness"]
+        self._taint_sink_replace_other = self._aspects_spec["taint_sinks"]["other"]
+        self._taint_sink_replace_any = self._taint_sink_replace_random.union(self._taint_sink_replace_other)
+        self._taint_sink_replace_disabled = self._aspects_spec["taint_sinks"]["disabled"]
+
         self.dont_patch_these_functionsdefs = set()
         for _, v in iteritems(self.excluded_functions):
             if v:
@@ -117,6 +150,16 @@ def __init__(
         # replacements and enabled again on all the others
         self.replacements_disabled_for_functiondef = False
 
+        self.codetype = CODE_TYPE_FIRST_PARTY
+        if "ast/tests/fixtures" in self.filename:
+            self.codetype = CODE_TYPE_FIRST_PARTY
+        elif "ddtrace" in self.filename and ("site-packages" in self.filename or "dist-packages" in self.filename):
+            self.codetype = CODE_TYPE_DD
+        elif "site-packages" in self.filename or "dist-packages" in self.filename:
+            self.codetype = CODE_TYPE_SITE_PACKAGES
+        elif "lib/python" in self.filename:
+            self.codetype = CODE_TYPE_STDLIB
+
     def _is_string_node(self, node):  # type: (Any) -> bool
         if PY30_37 and isinstance(node, ast.Bytes):
             return True
@@ -155,6 +198,42 @@ def _is_string_format_with_literals(self, call_node):
             and all(map(lambda x: self._is_node_constant_or_binop(x.value), call_node.keywords))
         )
 
+    def _get_function_name(self, call_node, is_function):  # type: (ast.Call, bool) -> str
+        if is_function:
+            return call_node.func.id  # type: ignore[attr-defined]
+        # If the call is to a method
+        elif type(call_node.func) == ast.Name:
+            return call_node.func.id
+
+        return call_node.func.attr  # type: ignore[attr-defined]
+
+    def _should_replace_with_taint_sink(self, call_node, is_function):  # type: (ast.Call, bool) -> bool
+        function_name = self._get_function_name(call_node, is_function)
+
+        if function_name in self._taint_sink_replace_disabled:
+            return False
+
+        return any(allowed in function_name for allowed in self._taint_sink_replace_any)
+
+    def _add_original_function_as_arg(self, call_node, is_function):  # type: (ast.Call, bool) -> Any
+        """
+        Creates the arguments for the original function
+        """
+        function_name = self._get_function_name(call_node, is_function)
+        function_name_arg = (
+            self._name_node(call_node, function_name, ctx=ast.Load()) if is_function else copy.copy(call_node.func)
+        )
+
+        # Arguments for stack info change from:
+        # my_function(self, *args, **kwargs)
+        # to:
+        # _add_original_function_as_arg(function_name=my_function, self, *args, **kwargs)
+        new_args = [
+            function_name_arg,
+        ] + call_node.args
+
+        return new_args
+
     def _node(self, type_, pos_from_node, **kwargs):
         # type: (Any, Any, Any) -> Any
         """
@@ -361,6 +440,24 @@ def visit_Call(self, call_node):  # type: (ast.Call) -> Any
                     call_node.func = self._attr_node(call_node, aspect)
                     self.ast_modified = call_modified = True
 
+        if self.codetype == CODE_TYPE_FIRST_PARTY:
+            # Function replacement case
+            if isinstance(call_node.func, ast.Name):
+                aspect = self._should_replace_with_taint_sink(call_node, True)
+                if aspect:
+                    call_node.args = self._add_original_function_as_arg(call_node, False)
+                    call_node.func = self._attr_node(call_node, TAINT_SINK_FUNCTION_REPLACEMENT)
+                    self.ast_modified = call_modified = True
+
+            # Method replacement case
+            elif isinstance(call_node.func, ast.Attribute):
+                aspect = self._should_replace_with_taint_sink(call_node, False)
+                if aspect:
+                    # Create a new Name node for the replacement and set it as node.func
+                    call_node.args = self._add_original_function_as_arg(call_node, False)
+                    call_node.func = self._attr_node(call_node, TAINT_SINK_FUNCTION_REPLACEMENT)
+                    self.ast_modified = call_modified = True
+
         if call_modified:
             _set_metric_iast_instrumented_propagation()
 

ddtrace/appsec/_iast/constants.py

@@ -2,6 +2,7 @@
 VULN_WEAK_CIPHER_TYPE = "WEAK_CIPHER"
 VULN_SQL_INJECTION = "SQL_INJECTION"
 VULN_PATH_TRAVERSAL = "PATH_TRAVERSAL"
+VULN_WEAK_RANDOMNESS = "WEAK_RANDOMNESS"
 VULN_INSECURE_COOKIE = "INSECURE_COOKIE"
 VULN_NO_HTTPONLY_COOKIE = "NO_HTTPONLY_COOKIE"
 VULN_NO_SAMESITE_COOKIE = "NO_SAMESITE_COOKIE"
@@ -11,6 +12,7 @@
 EVIDENCE_ALGORITHM_TYPE = "ALGORITHM"
 EVIDENCE_SQL_INJECTION = "SQL_INJECTION"
 EVIDENCE_PATH_TRAVERSAL = "PATH_TRAVERSAL"
+EVIDENCE_WEAK_RANDOMNESS = "WEAK_RANDOMNESS"
 EVIDENCE_COOKIE = "COOKIE"
 EVIDENCE_CMDI = "COMMAND"
 
@@ -28,3 +30,25 @@
 DEFAULT_WEAK_HASH_ALGORITHMS = {MD5_DEF, SHA1_DEF}
 
 DEFAULT_WEAK_CIPHER_ALGORITHMS = {DES_DEF, BLOWFISH_DEF, RC2_DEF, RC4_DEF, IDEA_DEF}
+
+DEFAULT_WEAK_RANDOMNESS_FUNCTIONS = {
+    "random",
+    "randint",
+    "randrange",
+    "choice",
+    "shuffle",
+    "betavariate",
+    "gammavariate",
+    "expovariate",
+    "choices",
+    "gauss",
+    "uniform",
+    "lognormvariate",
+    "normalvariate",
+    "paretovariate",
+    "sample",
+    "triangular",
+    "vonmisesvariate",
+    "weibullvariate",
+    "randbytes",
+}

ddtrace/appsec/_iast/taint_sinks/__init__.py

@@ -1,6 +1,8 @@
-from ddtrace.appsec._iast.taint_sinks.path_traversal import open_path_traversal
+from .ast_taint import ast_funcion
+from .path_traversal import open_path_traversal
 
 
 __all__ = [
     "open_path_traversal",
+    "ast_funcion",
 ]

ddtrace/appsec/_iast/taint_sinks/ast_taint.py

@@ -0,0 +1,31 @@
+from typing import TYPE_CHECKING
+
+from ..constants import DEFAULT_WEAK_RANDOMNESS_FUNCTIONS
+from .weak_randomness import WeakRandomness
+
+
+if TYPE_CHECKING:
+    from typing import Any
+    from typing import Callable
+
+
+def ast_funcion(
+    func,  # type: Callable
+    *args,  # type: Any
+    **kwargs  # type: Any
+):  # type: (...) -> Any
+
+    cls = getattr(func, "__self__", None)
+    func_name = getattr(func, "__name__", None)
+    cls_name = ""
+    if cls and func_name:
+        try:
+            cls_name = cls.__class__.__name__
+        except AttributeError:
+            pass
+
+    if cls.__class__.__module__ == "random" and cls_name == "Random" and func_name in DEFAULT_WEAK_RANDOMNESS_FUNCTIONS:
+        # Weak, run the analyzer
+        WeakRandomness.report(evidence_value=cls_name + "." + func_name)
+
+    return func(*args, **kwargs)

ddtrace/appsec/_iast/taint_sinks/weak_randomness.py

@@ -0,0 +1,14 @@
+from .. import oce
+from ..constants import EVIDENCE_WEAK_RANDOMNESS
+from ..constants import VULN_WEAK_RANDOMNESS
+from ._base import VulnerabilityBase
+
+
+@oce.register
+class WeakRandomness(VulnerabilityBase):
+    vulnerability_type = VULN_WEAK_RANDOMNESS
+    evidence_type = EVIDENCE_WEAK_RANDOMNESS
+
+    @classmethod
+    def report(cls, evidence_value=None, sources=None):
+        super(WeakRandomness, cls).report(evidence_value=evidence_value)

ddtrace/appsec/iast/_ast/__init__.py

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    Vulnerability Management for Code-level (IAST): Weak randomness vulnerability detection.