chore(asm): fix ddwaf errors found in 1.8.0rc1 [backport #4975 to 1.8] (#4991)

## Description

- Avoid any errors or exceptions raised while running the WAF to
interrupt the request
- Add test coverage and regression tests

## Checklist
- [ ] Followed the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines)
when writing a release note.
- [ ] Add additional sections for `feat` and `fix` pull requests.
- [ ] [Library
documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs)
and/or [Datadog's documentation
site](https://github.com/DataDog/documentation/) is updated. Link to doc
PR in description.

## Reviewer Checklist
- [ ] Title is accurate.
- [ ] Description motivates each change.
- [ ] No unnecessary changes were introduced in this PR.
- [ ] Avoid breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [ ] Tests provided or description of manual testing performed is
included in the code or PR.
- [ ] Release note has been added and follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines),
or else `changelog/no-changelog` label added.
- [ ] All relevant GitHub issues are correctly linked.
- [ ] Change contains telemetry where appropriate (logs, metrics, etc.).
- [ ] Telemetry is meaningful, actionable and does not have the
potential to leak sensitive data.

---------

Co-authored-by: Christophe Papazian <[email protected]>

Co-authored-by: Federico Mon <[email protected]>
Co-authored-by: Christophe Papazian <[email protected]>

Author: 3 people

ddtrace/appsec/ddwaf/__init__.py

@@ -24,11 +24,12 @@
     from .ddwaf_types import ddwaf_get_version
     from .ddwaf_types import ddwaf_init
     from .ddwaf_types import ddwaf_object
+    from .ddwaf_types import ddwaf_result
     from .ddwaf_types import ddwaf_result_free
     from .ddwaf_types import ddwaf_ruleset_info
+    from .ddwaf_types import ddwaf_run
     from .ddwaf_types import ddwaf_update_rule_data
     from .ddwaf_types import py_ddwaf_required_addresses
-    from .ddwaf_types import py_ddwaf_run
 
     _DDWAF_LOADED = True
 except OSError:
@@ -109,18 +110,27 @@ def run(
 
             ctx = ddwaf_context_init(self._handle)
             if ctx == 0:
-                raise RuntimeError
+                LOGGER.error("DDWaf failure to create the context")
+                return DDWaf_result(None, [], 0, (time.time() - start) * 1e6)
+
             try:
+                result = ddwaf_result()
                 wrapper = ddwaf_object(data)
-                error, result = py_ddwaf_run(ctx, wrapper, timeout_ms * 1000)
-                return DDWaf_result(
-                    result.data.decode("UTF-8") if result.data else None,
-                    [result.actions.array[i].decode("UTF-8") for i in range(result.actions.size)],
-                    result.total_runtime / 1e3,
-                    (time.time() - start) * 1e6,
-                )
+                error = ddwaf_run(ctx, wrapper, ctypes.byref(result), timeout_ms * 1000)
+                if error:
+                    LOGGER.warning("DDWAF error: %d", error)
+                try:
+                    return DDWaf_result(
+                        result.data.decode("UTF-8", errors="ignore")
+                        if hasattr(result, "data") and result.data
+                        else None,
+                        [result.actions.array[i].decode("UTF-8", errors="ignore") for i in range(result.actions.size)],
+                        result.total_runtime / 1e3,
+                        (time.time() - start) * 1e6,
+                    )
+                finally:
+                    ddwaf_result_free(ctypes.byref(result))
             finally:
-                ddwaf_result_free(ctypes.byref(result))
                 ddwaf_context_destroy(ctx)
 
         def __dealloc__(self):

ddtrace/appsec/ddwaf/ddwaf_types.py

@@ -108,9 +108,7 @@ class ddwaf_object(ctypes.Structure):
 
     def __init__(self, struct=None):
         # type: (ddwaf_object, DDWafRulesType|None) -> None
-        if struct is None:
-            ddwaf_object_invalid(self)
-        elif isinstance(struct, (int, long)):
+        if isinstance(struct, (int, long)):
             ddwaf_object_signed(self, struct)
         elif isinstance(struct, unicode):
             ddwaf_object_string(self, struct.encode("UTF-8", errors="ignore"))
@@ -130,15 +128,18 @@ def __init__(self, struct=None):
             map_o = ddwaf_object_map(self)
             assert map_o
             # order is unspecified and could lead to problems if max_objects is reached
-            for (key, val) in struct.items():
+            for key, val in struct.items():
                 if not isinstance(key, (bytes, unicode)):  # discards non string keys
                     continue
                 res_key = key.encode("UTF-8", errors="ignore") if isinstance(key, unicode) else key
                 obj = ddwaf_object(val)
                 if obj.type:  # discards invalid objects
                     assert ddwaf_object_map_add(map_o, res_key, obj)
         else:
-            raise TypeError("ddwaf_object : unknown type in structure. " + repr(type(struct)))
+            if struct is not None:
+                log.warning("DDWAF object init called with unknown data structure: %s", repr(type(struct)))
+
+            ddwaf_object_invalid(self)
 
     @property
     def struct(self):
@@ -151,17 +152,18 @@ def struct(self):
         if self.type == DDWAF_OBJ_TYPE.DDWAF_OBJ_UNSIGNED:
             return self.value.uintValue
         if self.type == DDWAF_OBJ_TYPE.DDWAF_OBJ_STRING:
-            return self.value.stringValue.decode("UTF-8")
+            return self.value.stringValue.decode("UTF-8", errors="ignore")
         if self.type == DDWAF_OBJ_TYPE.DDWAF_OBJ_ARRAY:
             return [self.value.array[i].struct for i in range(self.nbEntries)]
         if self.type == DDWAF_OBJ_TYPE.DDWAF_OBJ_MAP:
             return {
-                self.value.array[i].parameterName.decode("UTF-8"): self.value.array[i].struct
+                self.value.array[i].parameterName.decode("UTF-8", errors="ignore"): self.value.array[i].struct
                 for i in range(self.nbEntries)
             }
         if self.type == DDWAF_OBJ_TYPE.DDWAF_OBJ_BOOL:
             return self.value.boolean
-        raise ValueError("ddwaf_object: unknown object")
+        log.warning("ddwaf_object struct: unknown object type: %s", repr(type(self.type)))
+        return None
 
     def __repr__(self):
         return repr(self.struct)
@@ -373,14 +375,6 @@ def py_ddwaf_required_rule_data_ids(handle):
     ("ddwaf_run", ddwaf), ((1, "context"), (1, "data"), (1, "result"), (1, "timeout"))
 )
 
-
-def py_ddwaf_run(context, object_p, timeout):
-    # type : (...) -> tuple[int, ddwaf_result]
-    res = ddwaf_result()
-    err = ddwaf_run(context, object_p, ctypes.byref(res), timeout)
-    return err, res
-
-
 ddwaf_context_destroy = ctypes.CFUNCTYPE(None, ddwaf_context)(
     ("ddwaf_context_destroy", ddwaf),
     ((1, "context"),),

ddtrace/appsec/processor.py

@@ -34,6 +34,12 @@
 from ddtrace.internal.rate_limiter import RateLimiter
 
 
+try:
+    from json.decoder import JSONDecodeError
+except ImportError:
+    # handling python 2.X import error
+    JSONDecodeError = ValueError  # type: ignore
+
 if TYPE_CHECKING:  # pragma: no cover
     from typing import Dict
 
@@ -180,7 +186,7 @@ def __attrs_post_init__(self):
                     # TODO: try to log reasons
                     log.error("[DDAS-0001-03] AppSec could not read the rule file %s.", self.rules)
                 raise
-            except json.decoder.JSONDecodeError:
+            except JSONDecodeError:
                 log.error(
                     "[DDAS-0001-03] AppSec could not read the rule file %s. Reason: invalid JSON file", self.rules
                 )
@@ -270,7 +276,13 @@ def on_span_finish(self, span):
                 data[_Addresses.SERVER_REQUEST_BODY] = body
 
         log.debug("[DDAS-001-00] Executing AppSec In-App WAF with parameters: %s", data)
-        ddwaf_result = self._ddwaf.run(data, self._waf_timeout)  # res is a serialized json
+        ddwaf_result = None
+        try:
+            ddwaf_result = self._ddwaf.run(data, self._waf_timeout)  # res is a serialized json
+        except OSError:
+            log.warning("Error executing Appsec In-App WAF: ", exc_info=True)
+        except Exception as e:
+            log.warning("Error executing Appsec In-App WAF: %s", repr(e))
 
         try:
             info = self._ddwaf.info
@@ -281,13 +293,15 @@ def on_span_finish(self, span):
 
             span.set_metric(APPSEC_EVENT_RULE_LOADED, info.loaded)
             span.set_metric(APPSEC_EVENT_RULE_ERROR_COUNT, info.failed)
-            span.set_metric(APPSEC_WAF_DURATION, ddwaf_result.runtime)
-            span.set_metric(APPSEC_WAF_DURATION_EXT, ddwaf_result.total_runtime)
-        except (json.decoder.JSONDecodeError, ValueError):
+            if ddwaf_result:
+                span.set_metric(APPSEC_WAF_DURATION, ddwaf_result.runtime)
+                span.set_metric(APPSEC_WAF_DURATION_EXT, ddwaf_result.total_runtime)
+        except JSONDecodeError:
             log.warning("Error parsing data AppSec In-App WAF metrics report")
         except Exception:
             log.warning("Error executing AppSec In-App WAF metrics report: %s", exc_info=True)
-        if ddwaf_result.data is not None:
+
+        if ddwaf_result and ddwaf_result.data is not None:
             # We run the rate limiter only if there is an attack, its goal is to limit the number of collected asm
             # events
             allowed = self._rate_limiter.is_allowed(span.start_ns)

tests/appsec/test_processor.py

@@ -1,6 +1,8 @@
 import json
+import logging
 import os.path
 
+import mock
 import pytest
 from six import ensure_binary
 
@@ -20,6 +22,12 @@
 from tests.utils import snapshot
 
 
+try:
+    from json.decoder import JSONDecodeError
+except ImportError:
+    # handling python 2.X import error
+    JSONDecodeError = ValueError  # type: ignore
+
 ROOT_DIR = os.path.dirname(os.path.abspath(__file__))
 RULES_GOOD_PATH = os.path.join(ROOT_DIR, "rules-good.json")
 RULES_BAD_PATH = os.path.join(ROOT_DIR, "rules-bad.json")
@@ -423,3 +431,115 @@ def test_ddwaf_info_with_3_errors():
         assert info.loaded == 1
         assert info.failed == 2
         assert info.errors == {"missing key 'name'": ["crs-942-100", "crs-913-120"]}
+
+
+def test_ddwaf_info_with_json_decode_errors(tracer_appsec, caplog):
+    tracer = tracer_appsec
+    config = Config()
+    config.http_tag_query_string = True
+
+    with caplog.at_level(logging.WARNING), override_env(dict(DD_TRACE_CLIENT_IP_HEADER_DISABLED="False")), mock.patch(
+        "ddtrace.appsec.processor.json.dumps", side_effect=JSONDecodeError("error", "error", 0)
+    ), mock.patch.object(DDWaf, "info"):
+        with tracer.trace("test", span_type=SpanTypes.WEB) as span:
+            set_http_meta(
+                span,
+                config,
+                method="PATCH",
+                url=u"http://localhost/api/unstable/role_requests/dab1e9ae-9d99-11ed-bfdf-da7ad0900000?_authentication_token=2b0297348221f294de3a047e2ecf1235abb866b6",  # noqa: E501
+                status_code="200",
+                raw_uri=u"http://localhost/api/unstable/role_requests/dab1e9ae-9d99-11ed-bfdf-da7ad0900000?_authentication_token=2b0297348221f294de3a047e2ecf1235abb866b6",  # noqa: E501
+                request_headers={
+                    "host": u"localhost",
+                    "user-agent": "aa",
+                    "content-length": u"73",
+                },
+                response_headers={
+                    "content-length": "501",
+                    "x-ratelimit-remaining": "363",
+                    "x-ratelimit-name": "role_api",
+                    "x-ratelimit-limit": "500",
+                    "x-ratelimit-period": "60",
+                    "content-type": "application/json",
+                    "x-ratelimit-reset": "16",
+                },
+                request_body={"_authentication_token": u"2b0297348221f294de3a047e2ecf1235abb866b6"},
+            )
+
+    assert "Error parsing data AppSec In-App WAF metrics report" in caplog.text
+
+
+def test_ddwaf_run_contained_typeerror(tracer_appsec, caplog):
+    tracer = tracer_appsec
+
+    config = Config()
+    config.http_tag_query_string = True
+
+    with caplog.at_level(logging.WARNING), mock.patch(
+        "ddtrace.appsec.ddwaf.ddwaf_run", side_effect=TypeError("expected c_long instead of int")
+    ), override_env(dict(DD_TRACE_CLIENT_IP_HEADER_DISABLED="False")):
+        with tracer.trace("test", span_type=SpanTypes.WEB) as span:
+            set_http_meta(
+                span,
+                config,
+                method="PATCH",
+                url=u"http://localhost/api/unstable/role_requests/dab1e9ae-9d99-11ed-bfdf-da7ad0900000?_authentication_token=2b0297348221f294de3a047e2ecf1235abb866b6",  # noqa: E501
+                status_code="200",
+                raw_uri=u"http://localhost/api/unstable/role_requests/dab1e9ae-9d99-11ed-bfdf-da7ad0900000?_authentication_token=2b0297348221f294de3a047e2ecf1235abb866b6",  # noqa: E501
+                request_headers={
+                    "host": u"localhost",
+                    "user-agent": "aa",
+                    "content-length": u"73",
+                },
+                response_headers={
+                    "content-length": "501",
+                    "x-ratelimit-remaining": "363",
+                    "x-ratelimit-name": "role_api",
+                    "x-ratelimit-limit": "500",
+                    "x-ratelimit-period": "60",
+                    "content-type": "application/json",
+                    "x-ratelimit-reset": "16",
+                },
+                request_body={"_authentication_token": u"2b0297348221f294de3a047e2ecf1235abb866b6"},
+            )
+
+    assert span.get_tag(APPSEC_JSON) is None
+    assert "Error executing Appsec In-App WAF: TypeError('expected c_long instead of int'" in caplog.text
+
+
+def test_ddwaf_run_contained_oserror(tracer_appsec, caplog):
+    tracer = tracer_appsec
+
+    config = Config()
+    config.http_tag_query_string = True
+
+    with caplog.at_level(logging.WARNING), mock.patch(
+        "ddtrace.appsec.ddwaf.ddwaf_run", side_effect=OSError("ddwaf run failed")
+    ), override_env(dict(DD_TRACE_CLIENT_IP_HEADER_DISABLED="False")):
+        with tracer.trace("test", span_type=SpanTypes.WEB) as span:
+            set_http_meta(
+                span,
+                config,
+                method="PATCH",
+                url=u"http://localhost/api/unstable/role_requests/dab1e9ae-9d99-11ed-bfdf-da7ad0900000?_authentication_token=2b0297348221f294de3a047e2ecf1235abb866b6",  # noqa: E501
+                status_code="200",
+                raw_uri=u"http://localhost/api/unstable/role_requests/dab1e9ae-9d99-11ed-bfdf-da7ad0900000?_authentication_token=2b0297348221f294de3a047e2ecf1235abb866b6",  # noqa: E501
+                request_headers={
+                    "host": u"localhost",
+                    "user-agent": "aa",
+                    "content-length": u"73",
+                },
+                response_headers={
+                    "content-length": "501",
+                    "x-ratelimit-remaining": "363",
+                    "x-ratelimit-name": "role_api",
+                    "x-ratelimit-limit": "500",
+                    "x-ratelimit-period": "60",
+                    "content-type": "application/json",
+                    "x-ratelimit-reset": "16",
+                },
+                request_body={"_authentication_token": u"2b0297348221f294de3a047e2ecf1235abb866b6"},
+            )
+
+    assert span.get_tag(APPSEC_JSON) is None
+    assert "Error executing Appsec In-App WAF: \nTraceback (" in caplog.text