Update libddwaf to 2.0.1 - #206
Merged
Merged
Conversation
Major API migration: explicit allocators, ddwaf_object/ddwaf_object_kv layout change, ddwaf_run -> ddwaf_context_eval, ephemerals -> subcontexts, ddwaf_config removal (obfuscator config now via ddwaf_builder_add_or_update_config). - Bump submodule/libddwafVersion/Waf.LIB_VERSION to 2.0.1, build.gradle to 17.5.0 - Rewrite waf_jni.c/output.c/byte_buffer.c for the new C API and ddwaf_object layout - Add _Static_assert layout pinning (ddwaf_layout.h) plus a native/Java cross-check (ByteBufferSerializer.checkNativeLayout) so a future libddwaf bump fails loudly instead of corrupting memory - Rewrite ByteBufferSerializer's zero-copy binary layout for the new 16/32-byte ddwaf_object/ddwaf_object_kv structures - Reject combined persistent+ephemeral evaluation (no longer supported by libddwaf 2.x) - Fix _has_events() reading a nonexistent key, always-true bug - Cap container insertion loops at MAX_CONTAINER_SIZE to avoid uint16 overflow - Convert v1.0 ruleset test fixtures to schema 2.1 (v1.0 support removed upstream) - Quality pass: dedupe LSTR/duration-read helpers, initialize key_len before use in output.c accessors, guard obfuscator config writes, fix FLOAT->JSON inf/nan case, pre-size Map insertion, remove hidden mutable offset state in ByteBufferSerializer
- Fix clang-format-18 violations blocking CI (waf_jni.c) - Rename DDWAF_RUN_TIMEOUT to LIBDDWAF_JAVA_RUN_TIMEOUT to avoid colliding with a libddwaf-owned macro name - Fix wrong unit in comment (duration is nanoseconds, not microseconds) - Extract duplicated key-string-fallback block in output.c into a helper - Remove unnecessary FQN from WafContext javadoc - Add round-trip test comparing ByteBufferSerializer output against the official ddwaf_object_set_* C API, plus small-string/string boundary tests - Document why the MAX_CONTAINER_SIZE guard in writeArrayOrMap is currently unreachable - Zero out write-destination slots on failure across all ByteBufferSerializer write paths, as defense in depth against a stale pointer reaching libddwaf - Document the single-registrant assumption on libddwaf's global obfuscator config section - Raise container-truncation logging from INFO to WARN and note the 65535 cap is new in libddwaf 2.x
… rule result.attributes.isEmpty() triggers CodeNarc's UnnecessaryGetter rule (codenarcTest), which was failing CI (ASAN, Coverage, Development tests).
|
🎯 Code Coverage (details) 🔗 Commit SHA: 2c6ba9e | Docs | Datadog PR Page | Give us feedback! |
Member
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The 65535-entry uint16 cap introduced by libddwaf 2.x had no regression test. Also align the truncation log level with waf_jni.c (WARN, not DEBUG) so the new silent-truncation limit is actually visible.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Bumps the vendored
libddwaffrom 1.30.0 to 2.0.1 across all four versionsources of truth (submodule pointer,
libddwafVersionin.github/workflows/actions.yml,Waf.LIB_VERSION, and the artifact versionin
build.gradle), and rewrites the JNI binding to follow libddwaf's majorC API change.
Motivation
Jira ticket: APPSEC-69428
Description of the Change
libddwaf 2.x replaces the API surface this binding hand-rolls against:
ddwaf_context_init/ddwaf_object_destroynow takean allocator. Input objects keep using
alloc = NULL(view-only, zero-copy)config.free_fn = NULLpattern.ddwaf_object/ddwaf_object_kvlayout change.ddwaf_objectstays16 bytes; map entries now live in a new 32-byte
ddwaf_object_kvstruct(key + value side by side) instead of the key living on the child object.
ByteBufferSerializer's zero-copy binary layout was rewritten for this,with
_Static_asserts pinning every offset inddwaf_layout.hand aruntime cross-check (
ByteBufferSerializer.checkNativeLayout()) so afuture libddwaf bump fails loudly instead of silently corrupting memory.
ddwaf_run->ddwaf_context_eval. Persistent evaluation moves to thenew function; ephemeral evaluation is replaced entirely by
ddwaf_subcontext_init/eval/destroy(subcontexts), replacing the oldephemeral-buffer-per-call flow in
WafContext.ddwaf_configremoved. Obfuscator regex configuration now goes throughddwaf_builder_add_or_update_configinstead.uint16container size/capacity. Containers are capped at 65535entries (previously effectively unbounded); insertion loops in
waf_jni.c/ByteBufferSerializer.javanow clamp to this limit, logging atWARNwhen truncation actually occurs (a regression test now covers theJava-side clamp as well).
ddwaf_object_get_size/at_key/at_valueaccessors on the result-readingpath (
output.c).run(persistentData, ephemeralData, ...)now rejects the combinedpersistent+ephemeral case in Java, before any serialization happens
(libddwaf 2.x has no combined-evaluation mode).
removed upstream).
Additional Notes
Behavior changes worth flagging for consumers (e.g. dd-trace-java):
key_patharray indices in event JSON are now emitted as JSONnumbers instead of strings (matches libddwaf 2.x's own event schema; no
key was renamed). Confirmed via
ObfuscationTests.groovy, where theexpected
key_pathfor an array index changed from'0'to0. Anyconsumer that deserializes this JSON into a strictly-typed field (e.g.
List<String>) will need to update that type - please check beforeupgrading.
neither) now throws
InvalidArgumentWafExceptionearlier: during Javaserialization rather than after a native call. This path isn't reachable
through the public
run()/runEphemeral()API, only via the privatecombined-mode method, so it shouldn't affect existing callers.
hard limit from libddwaf 2.x's
uint16ddwaf_objectsize field); thisis logged at
WARN.This is a minor version bump (17.4.0 to 17.5.0), not a major one. No
Java method signature changed, and the one confirmed behavior change
(
key_pathindex encoding) fails loudly for any strictly-typed consumerrather than silently corrupting data, so it doesn't rise to the level of a
breaking change under this repo's versioning policy (see
AGENTS.md).That said, it's a real wire-format change, called out explicitly above so
downstream consumers can check their own parsing code before adopting this
version.