fix(iast): ensure args are not empty in aspects to avoid index error [backport 1.20] (#7740)

Backport 20ea51d from #7738 to 1.20.

IAST: Resolved an issue where certain aspects raised an IndexError when
no arguments were provided, despite the replaced methods/functions
accommodating calls without parameters. The fix eliminates the
requirement for at least one parameter in these aspects and includes
regression tests to ensure stability.

## 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]>
Co-authored-by: Alberto Vara <[email protected]>

Author: 3 people

.circleci/config.templ.yml

@@ -78,7 +78,7 @@ commands:
     description: "Install riot"
     steps:
       # Make sure we install and run riot on Python 3
-      - run: pip3 install riot==0.17.7
+      - run: pip3 install riot==0.17.7 'virtualenv<=20.24.6' 'setuptools<=68.2.0'
 
   setup_hatch:
     description: "Install hatch"

ddtrace/appsec/iast/_taint_tracking/aspects.py

@@ -61,7 +61,7 @@ def str_aspect(orig_function, flag_added_args, *args, **kwargs):
     else:
         result = args[0].str(*args[1:], **kwargs)
 
-    if isinstance(args[0], TEXT_TYPES) and is_pyobject_tainted(args[0]):
+    if args and isinstance(args[0], TEXT_TYPES) and is_pyobject_tainted(args[0]):
         try:
             if isinstance(args[0], (bytes, bytearray)):
                 check_offset = args[0].decode("utf-8")
@@ -87,7 +87,7 @@ def bytes_aspect(orig_function, flag_added_args, *args, **kwargs):
     else:
         result = args[0].bytes(*args[1:], **kwargs)
 
-    if isinstance(args[0], TEXT_TYPES) and is_pyobject_tainted(args[0]):
+    if args and isinstance(args[0], TEXT_TYPES) and is_pyobject_tainted(args[0]):
         try:
             taint_pyobject_with_ranges(result, tuple(get_ranges(args[0])))
         except Exception as e:
@@ -106,7 +106,7 @@ def bytearray_aspect(orig_function, flag_added_args, *args, **kwargs):
     else:
         result = args[0].bytearray(*args[1:], **kwargs)
 
-    if isinstance(args[0], TEXT_TYPES) and is_pyobject_tainted(args[0]):
+    if args and isinstance(args[0], TEXT_TYPES) and is_pyobject_tainted(args[0]):
         try:
             taint_pyobject_with_ranges(result, tuple(get_ranges(args[0])))
         except Exception as e:
@@ -123,6 +123,9 @@ def join_aspect(orig_function, flag_added_args, *args, **kwargs):
             args = args[flag_added_args:]
         return orig_function(*args, **kwargs)
 
+    if not args:
+        return orig_function(*args, **kwargs)
+
     joiner = args[0]
     args = args[flag_added_args:]
     if not isinstance(joiner, TEXT_TYPES):
@@ -292,6 +295,9 @@ def format_aspect(
             args = args[flag_added_args:]
         return orig_function(*args, **kwargs)
 
+    if not args:
+        return orig_function(*args, **kwargs)
+
     candidate_text = args[0]  # type: str
     args = args[flag_added_args:]
 
@@ -341,6 +347,9 @@ def format_map_aspect(
             args = args[flag_added_args:]
         return orig_function(*args, **kwargs)
 
+    if orig_function and not args:
+        return orig_function(*args, **kwargs)
+
     candidate_text = args[0]  # type: str
     args = args[flag_added_args:]
     if not isinstance(candidate_text, TEXT_TYPES):
@@ -385,7 +394,7 @@ def repr_aspect(orig_function, flag_added_args, *args, **kwargs):
 
     result = repr(*args, **kwargs)
 
-    if isinstance(args[0], TEXT_TYPES) and is_pyobject_tainted(args[0]):
+    if args and isinstance(args[0], TEXT_TYPES) and is_pyobject_tainted(args[0]):
         try:
             if isinstance(args[0], (bytes, bytearray)):
                 check_offset = args[0].decode("utf-8")
@@ -483,7 +492,7 @@ def incremental_translation(self, incr_coder, funcode, empty):
 
 
 def decode_aspect(orig_function, flag_added_args, *args, **kwargs):
-    if orig_function and not flag_added_args:
+    if orig_function and (not flag_added_args or not args):
         # This patch is unexpected, so we fallback
         # to executing the original function
         return orig_function(*args, **kwargs)
@@ -506,7 +515,7 @@ def decode_aspect(orig_function, flag_added_args, *args, **kwargs):
 
 
 def encode_aspect(orig_function, flag_added_args, *args, **kwargs):
-    if orig_function and not flag_added_args:
+    if orig_function and (not flag_added_args or not args):
         # This patch is unexpected, so we fallback
         # to executing the original function
         return orig_function(*args, **kwargs)
@@ -531,7 +540,7 @@ def encode_aspect(orig_function, flag_added_args, *args, **kwargs):
 def upper_aspect(
     orig_function, flag_added_args, *args, **kwargs
 ):  # type: (Optional[Callable], int, Any, Any) -> TEXT_TYPE
-    if orig_function and not isinstance(orig_function, BuiltinFunctionType):
+    if orig_function and (not isinstance(orig_function, BuiltinFunctionType) or not args):
         if flag_added_args > 0:
             args = args[flag_added_args:]
         return orig_function(*args, **kwargs)
@@ -551,7 +560,7 @@ def upper_aspect(
 def lower_aspect(
     orig_function, flag_added_args, *args, **kwargs
 ):  # type: (Optional[Callable], int, Any, Any) -> TEXT_TYPE
-    if orig_function and not isinstance(orig_function, BuiltinFunctionType):
+    if orig_function and (not isinstance(orig_function, BuiltinFunctionType) or not args):
         if flag_added_args > 0:
             args = args[flag_added_args:]
         return orig_function(*args, **kwargs)
@@ -571,7 +580,7 @@ def lower_aspect(
 def swapcase_aspect(
     orig_function, flag_added_args, *args, **kwargs
 ):  # type: (Optional[Callable], int, Any, Any) -> TEXT_TYPE
-    if orig_function and not isinstance(orig_function, BuiltinFunctionType):
+    if orig_function and (not isinstance(orig_function, BuiltinFunctionType) or not args):
         if flag_added_args > 0:
             args = args[flag_added_args:]
         return orig_function(*args, **kwargs)
@@ -590,7 +599,7 @@ def swapcase_aspect(
 def title_aspect(
     orig_function, flag_added_args, *args, **kwargs
 ):  # type: (Optional[Callable], int, Any, Any) -> TEXT_TYPE
-    if orig_function and not isinstance(orig_function, BuiltinFunctionType):
+    if orig_function and (not isinstance(orig_function, BuiltinFunctionType) or not args):
         if flag_added_args > 0:
             args = args[flag_added_args:]
         return orig_function(*args, **kwargs)
@@ -609,7 +618,7 @@ def title_aspect(
 def capitalize_aspect(
     orig_function, flag_added_args, *args, **kwargs
 ):  # type: (Optional[Callable], int, Any, Any) -> TEXT_TYPE
-    if orig_function and not isinstance(orig_function, BuiltinFunctionType):
+    if orig_function and (not isinstance(orig_function, BuiltinFunctionType) or not args):
         if flag_added_args > 0:
             args = args[flag_added_args:]
         return orig_function(*args, **kwargs)
@@ -630,7 +639,7 @@ def casefold_aspect(
     orig_function, flag_added_args, *args, **kwargs
 ):  # type: (Optional[Callable], int, Any, Any) -> TEXT_TYPE
     if orig_function:
-        if not isinstance(orig_function, BuiltinFunctionType):
+        if not isinstance(orig_function, BuiltinFunctionType) or not args:
             if flag_added_args > 0:
                 args = args[flag_added_args:]
             return orig_function(*args, **kwargs)
@@ -658,7 +667,7 @@ def casefold_aspect(
 def translate_aspect(
     orig_function, flag_added_args, *args, **kwargs
 ):  # type: (Optional[Callable], int, Any, Any) -> TEXT_TYPE
-    if orig_function and not isinstance(orig_function, BuiltinFunctionType):
+    if orig_function and (not isinstance(orig_function, BuiltinFunctionType) or not args):
         if flag_added_args > 0:
             args = args[flag_added_args:]
         return orig_function(*args, **kwargs)

releasenotes/notes/iast-fix-aspects-empty-parameters-d488c044d562a4cd.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    Vulnerability Management for Code-level (IAST): This fix resolves an issue where certain aspects incorrectly expected at least one argument, leading to an IndexError when none were provided. The solution removes this constraint and incorporates regression tests for stability assurance.

tests/appsec/iast/aspects/test_empty_arg_aspects.py

@@ -0,0 +1,118 @@
+# -*- coding: utf-8 -*-
+"""
+Common tests to aspects, ensuring that they don't break when receiving empty arguments.
+"""
+import os
+
+import pytest
+
+
+try:
+    # Disable unused import warning pylint: disable=W0611
+    from ddtrace.appsec._iast._taint_tracking import TagMappingMode  # noqa: F401
+except (ImportError, AttributeError):
+    pytest.skip("IAST not supported for this Python version", allow_module_level=True)
+
+from tests.appsec.iast.aspects.conftest import _iast_patched_module
+import tests.appsec.iast.fixtures.aspects.callees  # noqa: F401
+
+
+def generate_callers_from_callees(callers_file=""):
+    """
+    Generate a callers module from a callees module, calling all it's functions.
+    """
+    module_functions = [
+        "bytearray",
+        "bytes",
+        "str",
+    ]
+
+    str_functions = [
+        "casefold",
+        "capitalize",
+        "translate",
+        "title",
+        "swapcase",
+        "lower",
+        "upper",
+        "format_map",
+        "format",
+        "zfill",
+        "ljust",
+        "join",
+        "encode",
+    ]
+
+    with open(callers_file, "w", encoding="utf-8") as callers:
+        callers.write(
+            """
+import builtins as _builtins
+
+def call_builtin_repr(*args, **kwargs):
+    return _builtins.repr()\n
+
+def call_builtin_decode(*args, **kwargs):
+    return bytes.decode()\n
+            """
+        )
+        for function in str_functions:
+            callers.write(
+                f"""
+def call_builtin_{function}(*args, **kwargs):
+    return _builtins.str.{function}()\n
+                """
+            )
+
+        for function in module_functions:
+            callers.write(
+                f"""
+def callee_{function}(*args, **kwargs):
+    return {function}(*args, **kwargs)\n
+                """
+            )
+
+
+PATCHED_CALLERS_FILE = "tests/appsec/iast/fixtures/aspects/callers.py"
+UNPATCHED_CALLERS_FILE = "tests/appsec/iast/fixtures/aspects/unpatched_callers.py"
+
+for _file in (PATCHED_CALLERS_FILE, UNPATCHED_CALLERS_FILE):
+    generate_callers_from_callees(
+        callers_file=_file,
+    )
+
+patched_callers = _iast_patched_module(PATCHED_CALLERS_FILE.replace("/", ".")[0:-3])
+# This import needs to be done after the file is created (previous line)
+# pylint: disable=[wrong-import-position],[no-name-in-module]
+from tests.appsec.iast.fixtures.aspects import unpatched_callers  # type: ignore[attr-defined] # noqa: E402
+
+
+@pytest.mark.parametrize("aspect", [x for x in dir(unpatched_callers) if not x.startswith(("_", "@"))])
+def test_aspect_patched_result(aspect):
+    """
+    Test that the result of the patched aspect call is the same as the unpatched one.
+    """
+    if aspect.startswith("callee_"):
+        assert getattr(patched_callers, aspect)() == getattr(unpatched_callers, aspect)()
+    elif aspect.startswith("call_builtin_"):
+        try:
+            getattr(patched_callers, aspect)()
+        except TypeError as e:
+            aspect = aspect[len("call_builtin_") :]
+            if aspect == "repr":
+                assert e.args[0] == f"{aspect}() takes exactly one argument (0 given)"
+            else:
+                # assert exception message is "unbound method str.<aspect>() needs an argument"
+                aspect_namespace = "bytes" if aspect == "decode" else "str"
+                assert e.args[0] == f"unbound method {aspect_namespace}.{aspect}() needs an argument"
+
+
+def teardown_module(_):
+    """
+    Remove the callers file after the tests are done.
+    """
+    for _file in (PATCHED_CALLERS_FILE, UNPATCHED_CALLERS_FILE):
+        try:
+            os.remove(_file)
+        except FileNotFoundError:
+            pass
+        assert not os.path.exists(_file)